sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

dany.armstrong90 - Attacker can sell a position which is pending to close. #124

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

dany.armstrong90

high

Attacker can sell a position which is pending to close.

Title

Attacker can sell a position which is pending to close.

Summary

DelayedOrder.sol#announceLeverageClose function lock the position to close. But an attacker can unlock the locked position by calling LimitOrder.sol#cancelLimitOrder function. Thus attacker can sell a position which is pending to close.

Vulnerability Detail

DelayedOrder.sol#announceLeverageClose function is the following.

    function announceLeverageClose(uint256 tokenId, uint256 minFillPrice, uint256 keeperFee) external whenNotPaused {
        ......
        // Lock the NFT belonging to this position so that it can't be transferred to someone else.
        // Locking doesn't require an approval from the leverage trader.
        leverageModule.lock(tokenId);
        ......
    }

From comment, we can see that the token which is pending to close should not be transferred to someone else.

On the other hand, LimitOrder.sol#cancelLimitOrder function is the following.

    function cancelLimitOrder(uint256 tokenId) external {
        ......
        // Unlock the ERC721 position NFT to allow for transfers.
        ILeverageModule(vault.moduleAddress(FlatcoinModuleKeys._LEVERAGE_MODULE_KEY)).unlock(tokenId);
        ......
    }

So an attacker can unlock the position which is pending to close and sell it to someone else.

Example:

  1. Suppose that user1(attacker) owns tokenId1 position.
  2. Attacker set tokenId1 as pending to LimitClose by calling LimitOrder.sol#announceLimitOrder function.
  3. Next attacker set tokenId1 as pending to LeverageClose by calling DelayedOrder.sol#announceLeverageClose function.
  4. Next attacker unlock tokenId1 by calling LimitOrder.sol#cancelLimitOrder function.
  5. Next attacker sell tokenId1 to user2.
  6. After a short time, the pending order with LeverageClose type is executed by keeper and the margin of the position are sent to user1(attacker).

Impact

Attacker can sell a position which is pending to close. Thus the buyer of the position will lose fund deposited to buy the position.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LimitOrder.sol#L94

Tool used

Manual Review

Recommendation

We recommend that check if the position is already locked when locking a position.

Duplicate of #48

sherlock-admin commented 8 months ago

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

takarez commented:

invalid: no sell of nft; see issue 089