Tapioca DAO - hassan-truscova's results

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/main/contracts/twAML.sol#L6-L106

Vulnerability details

Impact

The FullMath abstract contract was taken from Uniswap v3-core. However, the original solidity version that was used was < 0.8.0, meaning that the execution didn't revert when an overflow was reached. This effectively means that when a phantom overflow (a multiplication and division where an intermediate value overflows 256 bits) occurs the execution will revert and the correct result won't be returned. The original library was designed in a way that could handle intermediate overflows.

The correct result isn't returned in this case and the execution gets reverted when a phantom overflows occurs because the solidity version is "pragma solidity ^0.8.18;".

Hence, The FullMath contract doesn't correctly handle the case when an intermediate value overflows 256 bits. This happens because an overflow is desired in this case but it's never reached.

Proof of Concept

function muldiv(
    uint256 a,
    uint256 b,
    uint256 denominator
) internal pure returns (uint256 result) {
    // Handle division by zero
    require(denominator > 0);

    // 512-bit multiply [prod1 prod0] = a * b
    // Compute the product mod 2**256 and mod 2**256 - 1
    // then use the Chinese Remainder Theorem to reconstruct
    // the 512 bit result. The result is stored in two 256
    // variables such that product = prod1 * 2**256 + prod0
    uint256 prod0; // Least significant 256 bits of the product
    uint256 prod1; // Most significant 256 bits of the product
    assembly {
        let mm := mulmod(a, b, not(0))
        prod0 := mul(a, b)
        prod1 := sub(sub(mm, prod0), lt(mm, prod0))
    }

    .....
    .....
    result = prod0 * inv;
    return result;
}

As noted in the FullMath library, it was designed in order to handle "phantom overflow" i.e., allows multiplication and division where an intermediate value overflows 256 bits. This means that the original library can handle statements like :

FullMath.mulDiv(type(uint256).max / 2, 3, 111));

This a feature of the library, as statements like this cannot be handled by solidity >0.8 directly as the execution will revert due to an overflow. Because the original library was created with solidity version <0.8.0 (which doesn't revert on overflows) this behaviour was allowed as the expected intermediatory overflow could be reached.

However, due to the fact that the FullMath Library was ported to solidity version 0.8.18 and above, which is >0.8, this operation would revert as the intermediate calculations would overflow, meaning that it can't handle those multiplication and division where an intermediate value overflows the 256 bits.

Tools Used

Manual review + in-house tool

Recommended Mitigation Steps

The fix is to mark the full body in an unchecked block, in order to leverage the fact that the original version was designed in order to allow the intermediate overflow. A modified version of the original Fullmath library that uses unchecked blocks to handle the intended overflow, can be found in the 0.8 branch of the Uniswap v3-core repo.

Assessed type

Under/Overflow