search

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

2 stars 1 forks source link

Attacker can steal the locked NFT in protocol because of lacking check in function `borrowToBuy()` #28

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#L436

Vulnerability details

Impact

In function borrowToBuy(), the borrower takes a loan offer and uses the funds to purchase NFT. 

/* Take the loan offer. */
_takeLoanOffer(offer, signature, lienId, loanAmount, collateralTokenId);

/* Lock token. */
offer.collection.transferFrom(msg.sender, address(this), collateralTokenId);

/* Take pool funds from lender. */
pool.withdrawFrom(offer.lender, address(this), loanAmount);

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

/* Send token out to buyer. */ 
///////////////////////////////////////////////////////////////////////////
// @audit execution NFT might not is offer.collection
offer.collection.transferFrom(address(this), msg.sender, execution.sell.order.tokenId);

/* Send surplus to borrower. */
SafeTransferLib.safeTransferETH(msg.sender, loanAmount - execution.value);

However, it is not guaranteed that the NFT borrower bought is from the same collection as offer.collection. As a result, an attacker can purchase a cheap NFT and steal an expensive locked NFT that has the same tokenId.

Proof of Concept

Consider the scenario

  1. Attacker saw that there is a Cryptopunk NFT currently locked in protocol as collateral. He plans to steal it.
  2. Attacker creates a loan offer with the collection is Cryptopunk.
  3. Attacker uses function borrowToBuy() to take this offer, deposit a Cryptopunks NFT and buy an NFT from Otherdeed collection that have the same id with the locked Cryptopunk NFT in step 1.

Function borrowToBuy() transfers the locked Cryptopunk to the attacker because it did not check that the collection of the purchased NFT is matched with the collection of offer.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding checks to ensure the offer collection and execution collection is the same.

code423n4 commented 1 year ago

Withdrawn by minhquanym