Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/seawater/src/maths/full_math.rs#L32-L51

Vulnerability details

Impact

mul_div() uses calculations that prevent overflow/underflow, which can cause problems in various places and not following the purpose of the original FullMath.mulDiv.

Proof of Concept

The mul_div() is performing a * b / c:

full_math.rs#L32-L51

pub fn _mul_div(a: U256, b: U256, mut denom_and_rem: U256) -> Result<(U256, bool), Error> {
    if denom_and_rem == U256::ZERO {
        return Err(Error::DenominatorIsZero);
    }

    let mut mul_and_quo = a.widening_mul::<256, 4, 512, 8>(b);

    unsafe {
        ruint::algorithms::div(mul_and_quo.as_limbs_mut(), denom_and_rem.as_limbs_mut());
    }

    let limbs = mul_and_quo.into_limbs();
    if limbs[4..] != [0_u64; 4] {
        return Err(Error::DenominatorIsLteProdOne);
    }

    let has_carry = denom_and_rem != U256::ZERO;

    Ok((U256::from_limbs_slice(&limbs[0..4]), has_carry))
}

It first does the multiplication via widening_mul() and then the division inside the unsafe block. But the widening_mul() implementation does not allow overflow.

https://docs.rs/alloy-primitives/0.7.6/alloy_primitives/aliases/type.U256.html#method.widening_mul

Even the fact that the division is in a unsafe block, the multiplication will not be able to overflow, which is not the case in the original Uniswap's FullMath library:

An overflow is expected there, and the entire operation is in the unchecked block:

https://github.com/Uniswap/v3-core/blob/6562c52e8f75f0c10f9deaf44861847585fc8129/contracts/libraries/FullMath.sol#L5-L6

https://github.com/Uniswap/v3-core/blob/6562c52e8f75f0c10f9deaf44861847585fc8129/contracts/libraries/FullMath.sol#L19

*/// @notice Facilitates multiplication and division that can have overflow of an intermediate value without any loss of precision
/// @dev Handles "phantom overflow" i.e., allows multiplication and division where an intermediate value overflows 256 bits*

mul_div is used a lot in the protocol and this bug can block many of the math operations - here are some examples where mul_div is used:

swap()
position update
all functions in sqrt_price_math()

The math in all these cases should also be in unchecked blocks, but the fix in mul_div will fix that, since this is the actual math operation, the same applies for mul_div_rounding_up.

NOTE: Furthermore, one of the Main Invariants in the README mentioned that - Superposition should follow the UniswapV3 math faithfully, which is not the case here, violating the invariant. https://github.com/code-423n4/2024-08-superposition?tab=readme-ov-file#main-invariants

Tools Used

Manual Review & The same problem was found here - https://github.com/sherlock-audit/2023-01-uxd-judging/issues/273

Recommended Mitigation Steps

Wrap the entire function inside unchecked/unsafe block, like the Uniswap 0.8 implementation - https://github.com/Uniswap/v3-core/blob/0.8/contracts/libraries/FullMath.sol#L14-L19

Assessed type

Math

code-423n4 / 2024-08-superposition-validation

operations in _mul_div should allow overflow/underflow #244