thank_you - Auction can't be extended when block.timestamp is at auction's end

[sherlock-admin4]

thank_you

medium

Auction can't be extended when block.timestamp is at auction's end

Summary

When a user places a bid on an auction, EnglishPeriodicAuctionInternal._isReadyForTransfer() is called. This function will return true when the block.timestamp equals the auction end time. However, the _placeBid() function allows bidders to place a bid and extend the auction when the block.timestamp equals the auction end time.

This logic error results in the user not being allowed to place a bid at the auction end time and extending the auction.

Vulnerability Detail

The EnglishPeriodicAuctionInternal._isReadyForTransfer() can be seen below:

function _isReadyForTransfer(uint256 tokenId) internal view returns (bool) {
    if (tokenId >= IStewardLicense(address(this)).maxTokenCount()) {
        return false;
    }
    // AUDIT: note that this logic will result in the function returning true.
    return block.timestamp >= _auctionEndTime(tokenId);
}

This function is called in EnglishPeriodicAuctionFacet.placeBid():

function placeBid(uint256 tokenId, uint256 bidAmount) external payable {
    require(
        _isAuctionPeriod(tokenId),
        'EnglishPeriodicAuction: can only place bid in auction period'
    );
    require(
        !_isReadyForTransfer(tokenId),
        'EnglishPeriodicAuction: auction is over and awaiting transfer'
    );
    require(
        IAllowlistReadable(address(this)).isAllowed(msg.sender),
        'EnglishPeriodicAuction: sender is not allowed to place bid'
    );

    _placeBid(tokenId, msg.sender, bidAmount, msg.value);
}

If the block.timestamp equals the auction end time, then _isReadyForTransfer() will return true, which will revert the placeBid() function.

This results in bidders at the auction end time from submitting new bids. This is in contradiction to the logic in _placeBid that allows bidders to extend the bid at the auction end time.

Impact

Bidders at the time of the auction end will not be able to bid on the auction and extend it.

Code Snippet

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/main/pco-art/contracts/auction/EnglishPeriodicAuctionInternal.sol?plain=1#L235-L241

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/main/pco-art/contracts/auction/facets/EnglishPeriodicAuctionFacet.sol?plain=1#L153-L168

Tool used

Manual Review

Recommendation

The _isReadyForTransfer function should update the operator logic to > to allow users to make bids at the auction end time and extend the auction.

function _isReadyForTransfer(uint256 tokenId) internal view returns (bool) {
    if (tokenId >= IStewardLicense(address(this)).maxTokenCount()) {
        return false;
    }
    //slither-disable-next-line timestamp
    return block.timestamp > _auctionEndTime(tokenId);
}

Duplicate of #89