wickie - Using transferFrom() can lock NFTs in a contract when settling auctions.

[sherlock-admin2]

wickie

Medium

Using transferFrom() can lock NFTs in a contract when settling auctions.

Summary

In NounsAuctionHouseV3.sol::_settleAuction(), transferFrom is used to transfer the nouns token to the winner. However, if the winner is a contract that does not handle ERC-721, the nouns token will be locked in the contract forever, and the winner won't be able to cancel his stream in StreamEscrow. This use is discouraged by Openzeppelin and safeTransferFrom() should be used instead. Ref : https://docs.openzeppelin.com/contracts/3.x/api/token/erc721

Root Cause

    function _settleAuction() internal {
        INounsAuctionHouseV3.AuctionV2 memory _auction = auctionStorage;
        .....
        if (_auction.bidder == address(0)) {
            nouns.burn(_auction.nounId);
        } else {
@>            nouns.transferFrom(address(this), _auction.bidder, _auction.nounId);
        }

Internal pre-conditions

A contract that cannot handle ERC-721 must win the auction.

External pre-conditions

No response

Attack Path

No response

Impact

This will lead to Nouns token being stuck in contracts, and winner wont be able to cancel the stream.

PoC

No response

Mitigation

Consider using safeTransferFrom() instead of transferFrom().