code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

`IndexPool.mint()` Unchecked arithmetic can overflow that allows stealing of almost all the funds in the pool #96

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/sushiswap/trident/blob/b415cddf7d1ec5f3b0166bf50f3d9ca9433877ba/contracts/pool/IndexPool.sol#L102-L106

/// @dev Mints LP tokens - should be called via the router after transferring `bento` tokens.
/// The router must ensure that sufficient LP tokens are minted by using the return value.
function mint(bytes calldata data) public override lock returns (uint256 liquidity) {
    (address recipient, uint256 toMint) = abi.decode(data, (address, uint256));

    uint120 ratio = uint120(_div(toMint, totalSupply));

    for (uint256 i = 0; i < tokens.length; i++) {
        address tokenIn = tokens[i];
        uint120 reserve = records[tokenIn].reserve;
        // @dev If token balance is '0', initialize with `ratio`.
        uint120 amountIn = reserve != 0 ? uint120(_mul(ratio, reserve)) : ratio;
        require(amountIn >= MIN_BALANCE, "MIN_BALANCE");
        // @dev Check Trident router has sent `amountIn` for skim into pool.
        unchecked {
            // @dev This is safe from overflow - only logged amounts handled.
            require(_balance(tokenIn) >= amountIn + reserve, "NOT_RECEIVED");
            records[tokenIn].reserve += amountIn;
        }
        emit Mint(msg.sender, tokenIn, amountIn, recipient);
    }
    _mint(recipient, toMint);
    liquidity = toMint;
}

amountIn + reserve can overflow when the input toMint is large enough.

Proof of Concept

Given:

An attacker can:

  1. Call IndexPool.mint() with toMint: 78463772476971226471282463269569508102237745068594430558000
  2. amountIn (line 99) will be 1329227995784915872903807060276030464
  3. amountIn + reserve (line 104) will be 99999999999999999995685888, and require(_balance(tokenIn) >= amountIn + reserve, "NOT_RECEIVED") will pass, and token reserve will be changed to 99999999999999999995685888
  4. The attacker will receive 78463772476971226471282463269569508102237745068594430558000 (7.846377247697122e+58) LP token
  5. Call IndexPool.burn()
  6. The attacker will receive 99999999999999999995685888 (~1e+26) tokenA and tokenB
  7. Only 4314112 wei of tokenA and tokenB remains in the pool

Impact

Almost all the funds in the pool can be stolen.

Recommendation

Move require(_balance(tokenIn) >= amountIn + reserve, "NOT_RECEIVED"); out of the unchecked block.

itsmetechjay commented 2 years ago

Withdrawn by warden. Per WatchPug: "I mistakenly submitted some issues of sushi to wild credit while having two tabs open at the same time."