sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

hash - possible overflow in _GeneralIntegrate #131

Closed sherlock-admin2 closed 10 months ago

sherlock-admin2 commented 10 months ago

hash

medium

possible overflow in _GeneralIntegrate

Summary

Normal multiplication is used without considering possibility of overflow in intermediate steps of calculation

Vulnerability Detail

Throughout the contracts, normal multiplication is done for computing expressions of the form (uint256 uint256 / uint256). Such type of multiplicaion doesn't handle intermediate overflow ie. the result of (uint256 uint256) overflowing uint256. As an extreme case this might happen in the calculate integral function:

    function _GeneralIntegrate(
        uint256 V0,
        uint256 V1,
        uint256 V2,
        uint256 i,
        uint256 k
    ) internal pure returns (uint256) {
        require(V0 > 0, "TARGET_IS_ZERO");
        uint256 fairAmount = i * (V1 - V2); // i*delta
        if (k == 0) {
            return fairAmount / DecimalMath.ONE;
        }
        uint256 V0V0V1V2 = DecimalMath.divFloor(V0 * V0 / V1, V2);
        uint256 penalty = DecimalMath.mulFloor(k, V0V0V1V2); // k(V0^2/V1/V2)
        return (DecimalMath.ONE - k + penalty) * fairAmount / DecimalMath.ONE2;
    }

uint256 max ~= 10^76 fairAmount max = (1e10 1e20) 1e18 == 48 decimals remaining = 28 - 18 (decimals of remainig penalty term) = 10 Penalty is V0^2 / V1 / V2. If the reserves drop very low the ratio can go over 10 decimals

Impact

If the reserves go very low, calculations will revert due to overflow

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/lib/DODOMath.sol#L29-L44

Tool used

Manual Review

Recommendation

Use a phantom overflow resistant method. Can refer this lib

nevillehuang commented 10 months ago

Invalid/Low severity, unlikely if not impossible that such extreme values will occur, given supply cap of stable coin tokens. Additionally, the fairAmount calculation is wrong.