code-423n4 / 2023-02-ethos-findings

6 stars 4 forks source link

An overflow may occur on an unsafe cast from uint256 to int256 #686

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/ActivePool.sol#L277 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L134 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L136 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L141 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L228 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L235 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L248 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L121 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L123

Vulnerability details

Impact

When a uint256 is casted to int256, it doesn't ensure overflow safety. so in the range of values of uint256 [0, 2^256-1] that is out of bound for int256 [-2^255, 2^255-1], it would lead to an overflow. meaning it would be impacted in the range of ]2^255-1, 2^256-1]. It would return values that is in the negative value range of int256 which is totally unexpected behavior, it could lead to breaking a lot of core logic for the subsystem where it could impact. the likelihood is low, but there could always be ways for it to actually happen, so eliminating such attack vector would be very crucial in a trustless environment.

Proof of Concept

Since casting any variable from uint256 to int256, in the range of input values of [2^255, 2^256-1], would lead for the result to underflow, we would use the following example to showcase how it could happen, you may paste the following script on remix Ide to try it out.

    // using compiler ^0.8.0
    function typeCastingOverflows(uint a) external pure returns(int) {
        return int256(a);
    }

Tools Used

Vs Code, Remix Ide

Recommended Mitigation Steps

using SafeCast.toInt256() implementation by Openzepplin to safely cast from uint256 to int256.

    function toInt256(uint256 value) internal pure returns (int256) {
        // Note: Unsafe cast below is okay because `type(int256).max` is guaranteed to be positive
        require(value <= uint256(type(int256).max), "SafeCast: value doesn't fit in an int256");
        return int256(value);
    }
c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid