Summary

The critical bug in the StakingPool contract is related to the handling of the totalStaked variable during the _updateStrategyRewards() function. This bug can lead to incorrect accounting of the total staked amount, potentially allowing for manipulation of the staking logic and endangering the integrity of the contract's financial operations.

Vulnerability Details

The updateStrategyRewards() function (which does access control check and then calls _updateStrategyRewards() internal function) is designed to update the rewards and fees based on balance changes in strategies since the last update. It iterates through a list of strategy indices, calculates the total rewards, and updates the totalStaked variable accordingly. The issue arises in the following lines of the _updateStrategyRewards() function:

Here, totalRewards is an int256 and totalStaked is casted to int256 as well in order to do the addition. If int256(totalStaked) + totalRewards < 0, casting it directly to uint256 introduces severe issue. This is because int256 in Solidity is representation by 2's complement, the MSB is the sign bit. When casting to uint256, the sign bit will be interpreted as the actual content of the value, so the end result is usually a lot larger than expected. For example, below is a quick PoC that you can test in Remix IDE:

The expected return value is uint256(-1) = 1, but actually the return value is 115792089237316195423570985008687907853269984665640564039457584007913129639935.

Impact

The accounting system that depends on totalStaked will be wrong if the scenario described above happens.

Tools Used

Manual review, Remix IDE

Recommendations

Implement an absolute value function: