sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

kevinkien - Front-Running Vulnerability due to Lack of TWAP Oracle in FlapperUniV2SwapOnly Contract #36

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

kevinkien

High

Front-Running Vulnerability due to Lack of TWAP Oracle in FlapperUniV2SwapOnly Contract

Summary

The FlapperUniV2SwapOnly contract is vulnerable to front-running attacks due to its reliance on a spot price oracle (pip). This allows malicious actors to manipulate prices and exploit transactions for their own benefit.

Vulnerability Detail

The contract exec function uses the current spot price from the pip oracle to determine the amount of GEM tokens to purchase in exchange for a given amount of DAI (lot). However, this spot price can be easily manipulated by front-runners who observe the pending transaction in the mempool. These attackers can quickly execute a similar trade before the original transaction is mined, causing the price to shift unfavorably and allowing them to profit from the price difference.

Impact

Front-running attacks can lead to significant financial losses for users of the FlapperUniV2SwapOnly contract. The attacker gains an unfair advantage by manipulating the price, while the user receives a worse exchange rate for their trade. This can undermine trust in the contract and discourage users from participating.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2SwapOnly.sol#L122-L123

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

Tool used

Manual Review

Recommendation

recommended to replace the spot price oracle with a Time-Weighted Average Price (TWAP) oracle. A TWAP oracle calculates the average price of an asset over a period, making it much more difficult for attackers to manipulate prices in a single block.

interface TWAPOracleLike {
    function consult(address token) external view returns (uint256 price, uint256 timestamp);
}

contract FlapperUniV2SwapOnly {
    // ... other variables

    TWAPOracleLike public twapOracle;

    // ... other functions

    function file(bytes32 what, address data) external auth {
        // ... 
        else if (what == "twapOracle") twapOracle = TWAPOracleLike(data);
        // ...
    }

    function exec(uint256 lot) external auth nonReentrant { 
        // ... (add minimum lot size check)

        // Use TWAP oracle
        (uint256 _price, ) = twapOracle.consult(gem);
        require(_buy >= lot * want / (_price * RAY / spotter.par()), "FlapperUniV2SwapOnly/insufficient-buy-amount"); 

        // ... (rest of the function logic)
    }
}
telome commented 1 month ago

Oracle manipulations are out of scope (from the competition rules: "Oracles are trusted to provide non-stale and correct information."). Moreover, the possibility of front-running the SBE swap is known and out of scope (from the competition rules: "The dss-flappers (SBE) solution assumes losing several percentages of funds by design during the Uniswap interactions due to slippage, MEV, sudden market movements, sandwich attacks, oracles imprecision and update resolution, etc.").