search

code-423n4 / 2023-03-mute-findings

2 stars 1 forks source link

Function `takeBid()` allows attacker to sell any collateral NFT that deposited through function `borrowToBuy()` #29

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-contest225/blob/af270a1fb366c8dda0fbbdd7d6ddaf6f0011992f/contracts/Core.sol#L432 https://github.com/code-423n4/2023-03-contest225/blob/af270a1fb366c8dda0fbbdd7d6ddaf6f0011992f/contracts/Core.sol#L519

Vulnerability details

Impact

Function borrowToBuy() is used by the borrower to take a loan offer and uses the funds to purchase NFT. However, even though it sends ETH along when calling function execute() to buy the requested NFT, it approves the collateral NFT to Blur for no reason. 

/* Execute marketplace order. */
offer.collection.approve(_DELEGATE, collateralTokenId);
_EXCHANGE.execute{ value: execution.value }(execution.sell, execution.buy);

As a result, given the approval is there, the attacker can use function takeBid() to sell any locked NFT that is previously approved to Blur.

/* Execute marketplace order. */
lien.collection.approve(_DELEGATE, lien.tokenId);

// @audit can sell token of other loan because it's approved in borrowToBuy()
_EXCHANGE.execute(execution.sell, execution.buy);

Proof of Concept

  1. Alice (victim) uses the function borrowToBuy() to buy an NFT with a Cryptopunk NFT. This Cryptopunk is locked as collateral for her loan and also be approved to _DELEGATE.
  2. Attacker calls takeBid() with Execution data to sell Alice's Cryptopunk. Since the approval is given in step 1, it will not fail and successfully sell Alice's collateral NFT.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider removing unnecessary approval of collateral tokens.

code423n4 commented 1 year ago

Withdrawn by minhquanym