[M-02] Unsafe downcasting can lead to errors

[ddimitrov22]

Impact: High because important data can be lost

Likelihood: Low because it will happen when very large amounts are used

Description

There are instances where uint256 is downcasted to a much smaller uint128. For example, the uint256 unoReward is then multiplied and downcasted to uint128. Let's take a look at SSIP::pendingUno:

    function pendingUno(address _to) external view returns (uint256 pending) {
        uint256 tokenSupply = IERC20(riskPool).totalSupply();
        ...
            uint256 unoReward = blocks * poolInfo.unoMultiplierPerBlock;
            accUnoPerShare = accUnoPerShare + uint128((unoReward * ACC_UNO_PRECISION) / tokenSupply); //@audit wrong downcasting
        }
        uint256 userBalance = userInfo[_to].amount;
        pending = (userBalance * uint256(accUnoPerShare)) / ACC_UNO_PRECISION - userInfo[_to].rewardDebt;
    }

This is problematic because if the calculation is bigger than uin128, then only the least significant 128 bits will be used. This can lead to loss of data. The downcasting in this example is unnecessary as the accUnoPerShare then is casted to uint256 within the pending calculation. The same applies to the updatePool function where we can see unsafe downcasting again:

    function updatePool() public override {
        ...
                uint256 unoReward = blocks * poolInfo.unoMultiplierPerBlock;
                poolInfo.accUnoPerShare = poolInfo.accUnoPerShare + uint128(((unoReward * ACC_UNO_PRECISION) / tokenSupply));//@audit unsafe downcasting
            }
           ...
    }

Recommendations

Be consistent with uints or use the SafeCast library to avoid losing any data and undesirable scenarios.

[wankhede04]

fixed at commit ab6376e