091.md

Great Clear Viper

Medium

unsafe use of transferfrom instead of safetransferfrom

Summary

https://github.com/sherlock-audit/2024-11-nounsdao/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/NounsAuctionHouseV2.sol#L284 the settleauction function uses transferfrom to transfer the nft to the auctionbidder that wins the auction, but issue can arise if the auctionbidder is a smart contract that do not implement the erc721 recieve interface, the transfer will succeed, but the token may become "stuck" in that contract. This is because transferFrom does not check whether the receiving contract can handle ERC 721 tokens, leading to loss of nft here is the code snippest

if (_auction.bidder == address(0)) { nouns.burn(_auction.nounId); } else { @> nouns.transferFrom(address(this), _auction.bidder, _auction.nounId); }

    if (_auction.amount > 0) {
        _safeTransferETHWithFallback(owner(), _auction.amount);
    }

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

when the nft is transfered to the auction bidder and the auction bidder is a smart contract that do not implement the erc721 recieve interface the nft may get lost

Impact

using transfer from instead of safetransferfrom to transfer nft can lead to lost of nft, if the auctionbidder is a smart contract that do not implement the erc721 recieve interface the nft will be stuck in the contract

PoC

No response

Mitigation

use safetransferfrom instead of transferfrom because safetransferfrom includes safety check which make sure the reciepient can handle the token