Github username: -- Twitter username: -- Submission hash (on-chain): 0x338e55fc6fc966b513e970aebc3d1e1f269e8b059ee300b88ecee2f21f0b75d2 Severity: low

Description:

Unbounded State Variables Pose Risk to Contract Stability

Description

The ProofOfHumanityExtended and CrossChainProofOfHumanity contracts contain state variables that can be modified by the governor without any upper bounds. Specifically, the sharedStakeMultiplier, winnerStakeMultiplier, loserStakeMultiplier, and transferCooldown variables can be set to arbitrarily large values. This lack of upper bounds could potentially lead to unexpected behavior, contract instability, or even render certain functionalities unusable.

Proof of Concept (PoC)

https://github.com/hats-finance/Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0/blob/master/contracts/extending-old/ProofOfHumanityExtended.sol#L639-L644

// File: ProofOfHumanityExtended.sol
// Lines 639-648
function changeStakeMultipliers(
    uint256 _sharedStakeMultiplier,
    uint256 _winnerStakeMultiplier,
    uint256 _loserStakeMultiplier
) external onlyGovernor {
    sharedStakeMultiplier = _sharedStakeMultiplier; // Unbounded assignment
    winnerStakeMultiplier = _winnerStakeMultiplier; // Unbounded assignment
    loserStakeMultiplier = _loserStakeMultiplier; // Unbounded assignment
    emit StakeMultipliersChanged(_sharedStakeMultiplier, _winnerStakeMultiplier, _loserStakeMultiplier);
}

https://github.com/hats-finance/Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0/blob/master/contracts/CrossChainProofOfHumanity.sol#L182-L183

// File: CrossChainProofOfHumanity.sol
// Lines 182-184
function setTransferCooldown(uint256 _transferCooldown) external onlyGovernor {
    transferCooldown = _transferCooldown; // Unbounded assignment
}

Revised Code File

// File: ProofOfHumanityExtended.sol
+ uint256 private constant MAX_STAKE_MULTIPLIER = 10000; // 100% in basis points

 function changeStakeMultipliers(
     uint256 _sharedStakeMultiplier,
     uint256 _winnerStakeMultiplier,
     uint256 _loserStakeMultiplier
 ) external onlyGovernor {
+    require(_sharedStakeMultiplier <= MAX_STAKE_MULTIPLIER, "Shared stake multiplier too high");
+    require(_winnerStakeMultiplier <= MAX_STAKE_MULTIPLIER, "Winner stake multiplier too high");
+    require(_loserStakeMultiplier <= MAX_STAKE_MULTIPLIER, "Loser stake multiplier too high");
     sharedStakeMultiplier = _sharedStakeMultiplier;
     winnerStakeMultiplier = _winnerStakeMultiplier;
     loserStakeMultiplier = _loserStakeMultiplier;
     emit StakeMultipliersChanged(_sharedStakeMultiplier, _winnerStakeMultiplier, _loserStakeMultiplier);
 }

// File: CrossChainProofOfHumanity.sol
+ uint256 private constant MAX_TRANSFER_COOLDOWN = 365 days; // Example: maximum 1 year cooldown

 function setTransferCooldown(uint256 _transferCooldown) external onlyGovernor {
+    require(_transferCooldown <= MAX_TRANSFER_COOLDOWN, "Transfer cooldown too high");
     transferCooldown = _transferCooldown;
 }

The proposed fix introduces upper bounds for the stake multipliers and transfer cooldown. The MAX_STAKE_MULTIPLIER is set to 10000 (100% in basis points), assuming the multipliers are used in percentage calculations. The MAX_TRANSFER_COOLDOWN is set to 365 days as an example, but this should be adjusted based on the specific requirements of the system. These changes ensure that even if a malicious or compromised governor attempts to set extreme values, they will be constrained within reasonable limits, maintaining the intended functionality and stability of the contract.

hats-finance / Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0