code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

Wrap around in `RayMath::fromRay()` , `RayMath::fromRay()` & `RayMath::toRay()` when max decimals are reached #298

Closed c4-bot-1 closed 8 months ago

c4-bot-1 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/RayMath.sol#L21 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/RayMath.sol#L40 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/RayMath.sol#L58

Vulnerability details

The functions RayMath::fromRay() , RayMath::fromRay() & RayMath::toRay() all use a variable called decimals_ratio.

The variable decimals_ratio is denominated in uint. Also known as an unsigned integer.

One of the core functions of the uint variable is that it can never be negative. However, if you take an unsigned 0 and subtract 1 from it, the result wraps around, leaving a very large number.

In the variable decimals_ratio the decimals ratio gets calculated and part of this calculation requires a variable named _decimals which gets deducted from the 27 decimals which Ray is denominated in throughout the functions RayMath::fromRay() , RayMath::fromRay() & RayMath::toRay().

However, if the _decimals variable is ever higher than 27, the decimals ratio will end up being a very big number and over-inflate the values of the respective calculations.

Impact

The decimals ratio variable can wrap around and lead to over-inflated calculations if the _decimals variable is ever higher than the highest it can be, which is 27.

Proof of Concept

Instance 01:

    function fromRay(uint256 _a, uint256 _decimals) internal pure returns (uint256 b) {
@>      uint256 decimals_ratio = 10 ** (27 - _decimals);
        assembly {
            b := div(_a, decimals_ratio)
        }
    }

Instance 02:

function fromRay(
        uint256 _a,
        uint256 _decimals,
        bool _roundUp
    ) internal pure returns (uint256 b) {
@>      uint256 decimals_ratio = 10 ** (27 - _decimals);
        assembly {
            b := div(_a, decimals_ratio)

            if and(eq(_roundUp, 1), gt(mod(_a, decimals_ratio), 0)) {
                b := add(b, 1)
            }
        }
    }

Instance 03:

function toRay(uint256 _a, uint256 _decimals) internal pure returns (uint256 b) {
@>      uint256 decimals_ratio = 10 ** (27 - _decimals);
        // to avoid overflow, b/decimals_ratio == _a
        assembly {
            b := mul(_a, decimals_ratio)

            if iszero(eq(div(b, decimals_ratio), _a)) {
                revert(0, 0)
            }
        }
    }

Tools Used

Manual Review, VSCodium

Recommended Mitigation Steps

Check if the _decimals variable is below the maximum decimal range.

Instance 01:

function fromRay(uint256 _a, uint256 _decimals) internal pure returns (uint256 b) {
++      require(_decimals < 27)
        uint256 decimals_ratio = 10 ** (27 - _decimals);
        assembly {
            b := div(_a, decimals_ratio)
        }
    }

Instance 02:

function fromRay(
        uint256 _a,
        uint256 _decimals,
        bool _roundUp
    ) internal pure returns (uint256 b) {
++      require(_decimals < 27)
        uint256 decimals_ratio = 10 ** (27 - _decimals);
        assembly {
            b := div(_a, decimals_ratio)

            if and(eq(_roundUp, 1), gt(mod(_a, decimals_ratio), 0)) {
                b := add(b, 1)
            }
        }
    }

Instance 03:

function toRay(uint256 _a, uint256 _decimals) internal pure returns (uint256 b) {
++      require(_decimals < 27)
        uint256 decimals_ratio = 10 ** (27 - _decimals);
        // to avoid overflow, b/decimals_ratio == _a
        assembly {
            b := mul(_a, decimals_ratio)

            if iszero(eq(div(b, decimals_ratio), _a)) {
                revert(0, 0)
            }
        }
    }

Assessed type

Decimal

c4-pre-sort commented 9 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 9 months ago

out-of-scope and solc 0.8 prevent overflow/underflow

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid