kevinkien - Reentrancy Vulnerability in `exec` Function

[sherlock-admin3]

kevinkien

High

Reentrancy Vulnerability in exec Function

Summary

The exec function in the FlapperUniV2 contract is susceptible to reentrancy attacks. This vulnerability could allow a malicious contract to re-enter the exec function before it completes, potentially draining funds from the contract.

Vulnerability Detail

The exec function performs multiple external calls:

• Token Transfer: It transfers DAI to the Uniswap pair contract GemLike(dai).transfer(address(pair), _sell);.
• Uniswap Swap: It calls the swap function on the Uniswap pair pair.swap(_amt0Out, _amt1Out, address(this), new bytes(0));.
• Token Transfer (Liquidity): It transfers DAI and GEM to the pair for liquidity minting GemLike(dai).transfer(address(pair), lot - _sell); and GemLike(gem).transfer(address(pair), _buy);.
• Liquidity Minting: It calls the mint function on the pair pair.mint(receiver);.

If any of these external contract calls are to a malicious contract, that contract could exploit this vulnerability by calling back into the exec function before the original call completes. This could allow the attacker to repeat these actions multiple times within a single transaction, potentially draining funds from the contract.

Impact

A successful reentrancy attack could lead to significant financial losses for the contract owner and users. The attacker could drain the contract's DAI and GEM balances, disrupting its intended functionality.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2.sol#L141-L164

    function exec(uint256 lot) external auth {
        // Check Amounts
        (uint256 _reserveDai, uint256 _reserveGem) = _getReserves();

        uint256 _sell = _getDaiToSell(lot, _reserveDai);

        uint256 _buy = _getAmountOut(_sell, _reserveDai, _reserveGem);
        require(_buy >= _sell * want / (uint256(pip.read()) * RAY / spotter.par()), "FlapperUniV2/insufficient-buy-amount");
        //

        // Swap
        GemLike(dai).transfer(address(pair), _sell);
        (uint256 _amt0Out, uint256 _amt1Out) = daiFirst ? (uint256(0), _buy) : (_buy, uint256(0));
        pair.swap(_amt0Out, _amt1Out, address(this), new bytes(0));
        //

        // Deposit
        GemLike(dai).transfer(address(pair), lot - _sell);
        GemLike(gem).transfer(address(pair), _buy);
        uint256 _liquidity = pair.mint(receiver);
        //

        emit Exec(lot, _sell, _buy, _liquidity);
    }

Tool used

Manual Review

Recommendation

Implement a reentrancy guard on the exec function

// Reentrancy guard
bool private locked;

modifier nonReentrant() {
    require(!locked, "FlapperUniV2/reentrancy");
    locked = true;
    _;
    locked = false;
}

function exec(uint256 lot) external auth nonReentrant {
    // ... (rest of the function logic)
}

[sunbreak1211]

All of the external calls are to constant/immutable addresses or to ones set by governance, so assumed non malicious. Also exec is authed, so only authorized callers can enter it.