howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/maths/full_math.rs#L54

Vulnerability details

Impact

The current version of mulDiv() function does not correspond to the function from the same library in UniswapV3 Math. This leads to a situation where using of such library function can result in the issues when making calculations in the core functionality of the protocol.

Proof of Concept

The library implements mulDiv() function the following way:

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


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))
}

Now let's take a look at the UniswapV3 functionality:

https://github.com/Uniswap/v3-core/blob/main/contracts/libraries/FullMath.sol#L14-106

As you can see here, the current version of the function misses several key elements including calculations and checks associated with computations of the product, usage of Chinese Remainder theorem and Newton-Raphson iteration (for precision).

As stated in the docs:

We should follow Uniswap V3's math faithfully.

Tools Used

Manual review.

Recommended Mitigation Steps

Consider implementing the mulDiv() functionality as it's done in this open-source library:

https://github.com/0xKitsune/uniswap-v3-math/blob/c46783bd0190ec68836345ac7a7104c335f8151e/src/full_math.rs#L8-102

Assessed type

Other

c4-judge commented 3 weeks ago

alex-ppg marked the issue as not a duplicate

alex-ppg commented 3 weeks ago

The submission claims that the mul_div implementation of the full_math file deviates from the Uniswap V3 implementation, however, no functional examples of a deviation have been demonstrated.