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

5 stars 3 forks source link

kevinkien - Improper Input Validation in the exec Function #22

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

kevinkien

Medium

Improper Input Validation in the exec Function

Summary

The exec function in the FlapperUniV2 contract lacks input validation for the lot parameter. This can lead to unexpected behavior and potential losses if a user provides an unreasonable or extreme lot size.

Vulnerability Detail

The exec function accepts a lot parameter, which represents the desired amount of liquidity tokens to mint. There are no checks to ensure that this value is within reasonable bounds. A user could potentially pass a very large or very small value for lot, which could have the following consequences:

Impact

The lack of input validation in the exec function opens up the possibility of unintended trades being executed. This could result in financial losses due to high slippage or inefficient use of gas.

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

add input validation to the exec function

function exec(uint256 lot, uint256 maxSlippage) external auth {
    require(lot >= MIN_LOT_SIZE, "FlapperUniV2/lot-too-small");
    require(lot <= MAX_LOT_SIZE, "FlapperUniV2/lot-too-large");

    // ... (rest of the function logic)
}
telome commented 3 months ago

The lot value is entirely determined by vow.bump and splitter.burn which are both assumed to be set appropriately by governance.