Lack of Input Validation in initiateNeuroEmissionMultiplierUpdate Function

[nathanogaga118]

Vulnerability Summary

The initiateNeuroEmissionMultiplierUpdate function in the ParanetNeuroIncentivesPool contract lacks input validation for the newMultiplier parameter. This oversight allows for the setting of unintended or malicious multiplier values, which can disrupt the reward distribution mechanism and potentially lead to financial exploitation.

Vulnerable Code

https://github.com/OriginTrail/dkg-evm-module/blob/main/contracts/paranets/ParanetNeuroIncentivesPool.sol

function initiateNeuroEmissionMultiplierUpdate(uint256 newMultiplier) external onlyVotersRegistrar {
if (!neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].finalized) {
neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].multiplier = newMultiplier;
neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].timestamp =
block.timestamp +
neuroEmissionMultiplierUpdateDelay;
} else {
neuroEmissionMultipliers.push(
ParanetLib.NeuroEmissionMultiplier({
multiplier: newMultiplier,
timestamp: block.timestamp + neuroEmissionMultiplierUpdateDelay,
finalized: false
})
);
}

emit NeuroEmissionMultiplierUpdateInitiated(
    neuroEmissionMultipliers[neuroEmissionMultipliers.length - 2].multiplier,
    newMultiplier,
    block.timestamp + neuroEmissionMultiplierUpdateDelay
);

}

Detailed Description

The initiateNeuroEmissionMultiplierUpdate function is responsible for updating the neuroEmissionMultiplier, which determines how much NEURO is released per TRAC spent. However, the function does not validate the newMultiplier input. This lack of validation can lead to several issues:

Unintended Multiplier Values: Without constraints, the multiplier can be set to zero, negative, or excessively large values, disrupting the reward distribution logic.

Operational Disruption: An incorrect multiplier can lead to either no rewards being distributed or excessive rewards being given out, affecting the contract's operational integrity.

Potential Exploits: A malicious actor with the appropriate permissions could set the multiplier to a value that disproportionately benefits them, leading to financial exploitation.

Lack of Constraints: The absence of sanity checks on the multiplier value means there is no safeguard against human error or malicious input.

Impact

Financial Loss: Incorrect multiplier values can lead to financial discrepancies, either by overpaying or underpaying rewards.
Operational Disruption: The reward distribution mechanism can be severely affected, leading to loss of trust and potential contract failure.

Exploitation Risk: Malicious actors could exploit the lack of validation to manipulate the reward system for personal gain.

Severity

Critical: The lack of input validation for a core parameter that affects reward distribution is a critical vulnerability. It can lead to significant financial loss and operational disruption if exploited.

Proof of Concept (PoC) Using Foundry's Forge

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "forge-std/Test.sol";
import "../src/ParanetNeuroIncentivesPool.sol"; // Adjust the import path as necessary

contract ParanetNeuroIncentivesPoolTest is Test {
ParanetNeuroIncentivesPool pool;

function setUp() public {
    // Initialize the contract with dummy data
    pool = new ParanetNeuroIncentivesPool(
        address(this), // hubAddress
        address(this), // paranetsRegistryAddress
        address(this), // knowledgeMinersRegistryAddress
        bytes32(0),    // paranetId
        1e12,          // tracToNeuroEmissionMultiplier
        1000,          // paranetOperatorRewardPercentage
        1000           // paranetIncentivizationProposalVotersRewardPercentage
    );
}

function testMultiplierUpdateWithoutValidation() public {
    // Attempt to set an invalid multiplier
   uint256 invalidMultiplier = 0; // Example of an unintended multiplier value
    pool.initiateNeuroEmissionMultiplierUpdate(invalidMultiplier);

    // Check if the multiplier was set to an invalid value
    uint256 currentMultiplier = pool.getEffectiveNeuroEmissionMultiplier(block.timestamp);
    assertEq(currentMultiplier, invalidMultiplier, "Multiplier should not be set to an invalid value");
}

}
Recommended Fix
To address the issue, implement input validation in the initiateNeuroEmissionMultiplierUpdate function to ensure the newMultiplier is within a reasonable and predefined range. Here's a suggested fix:

function initiateNeuroEmissionMultiplierUpdate(uint256 newMultiplier) external onlyVotersRegistrar {
require(newMultiplier > 0, "Multiplier must be greater than zero");
require(newMultiplier <= MAX_MULTIPLIER, "Multiplier exceeds maximum allowed value");

if (!neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].finalized) {
    neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].multiplier = newMultiplier;
    neuroEmissionMultipliers[neuroEmissionMultipliers.length - 1].timestamp =
        block.timestamp +
        neuroEmissionMultiplierUpdateDelay;
} else {
    neuroEmissionMultipliers.push(
        ParanetLib.NeuroEmissionMultiplier({
            multiplier: newMultiplier,
            timestamp: block.timestamp + neuroEmissionMultiplierUpdateDelay,
            finalized: false
        })
    );
}

emit NeuroEmissionMultiplierUpdateInitiated(
    neuroEmissionMultipliers[neuroEmissionMultipliers.length - 2].multiplier,
    newMultiplier,
    block.timestamp + neuroEmissionMultiplierUpdateDelay
);

}
In this fix, MAX_MULTIPLIER should be defined as a constant that represents the maximum reasonable value for the multiplier.

[nathanogaga118]

@u-hubar

[nathanogaga118]

new fixed code and mergable pull request https://github.com/nathanogaga118/dkg-evm-module/blob/main/contracts/paranets/ParanetNeuroIncentivesPool.sol