sherlock-audit / 2024-03-nouns-dao-2-judging

1 stars 0 forks source link

bareli - Incorrect WETH Handling #40

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

bareli

medium

Incorrect WETH Handling

Summary

Incorrect WETH Handling: If WETH interaction fails, funds could be lost.

Vulnerability Detail

function _settleAuction() internal { INounsAuctionHouse.Auction memory _auction = auction;

    require(_auction.startTime != 0, "Auction hasn't begun");
    require(!_auction.settled, 'Auction has already been settled');
    require(block.timestamp >= _auction.endTime, "Auction hasn't completed");

    auction.settled = true;

    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);
    }

    emit AuctionSettled(_auction.nounId, _auction.bidder, _auction.amount);
}

/**
 * @notice Transfer ETH. If the ETH transfer fails, wrap the ETH and try send it as WETH.
 */

@> function _safeTransferETHWithFallback(address to, uint256 amount) internal { if (!_safeTransferETH(to, amount)) { IWETH(weth).deposit{ value: amount }(); IERC20(weth).transfer(to, amount); } }

Impact

IERC20(weth).transfer(to, amount) has no check for success .

Code Snippet

https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/NounsAuctionHouse.sol#L249

Tool used

Manual Review

Recommendation

use safetransfer instead of transfer.

sherlock-admin3 commented 4 months ago

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

WangAudit commented:

low/info; it's essentially a sanity check