salvorio / salvor-contracts

1 stars 0 forks source link

[SLG-03M] Inexistent Prevention of Dutch Auction Overlap #24

Closed HKskn closed 3 months ago

HKskn commented 7 months ago

SLG-03M: Inexistent Prevention of Dutch Auction Overlap

Type Severity Location
Logical Fault SalvorLending.sol:L459, L572, L693

Description:

The newly introduced Dutch payment feature is simultaneously available with loan extensions as well as clearances despite the DutchAuction structure containing a duration variable.

Impact:

Loan extensions via the SalvorLending::extend become meaningless when Dutch auctions are available as a would-be loan extender can simply repay the original loan without any mark-up to unlock the NFT.

Example:

/**
 * @notice Allows a bidder to make a bid for a Dutch auction.
 * @param _nftContractAddress The address of the NFT contract.
 * @param _tokenId The token ID of the NFT being auctioned.
 */
function makeBidForDutchAuction(address _nftContractAddress, uint256 _tokenId)
public
whenNotPaused
nonReentrant
auctionStarted(_nftContractAddress, _tokenId)
{
    address lender = items[_nftContractAddress][_tokenId].lender;
    require(lender != address(0), "NFT is not deposited");
    uint256 price = getDutchPrice(_nftContractAddress, _tokenId);
    emit DutchAuctionMadeBid(_nftContractAddress, _tokenId, lender, price, dutchAuctions[_nftContractAddress][_tokenId].endPrice);

    IAssetManager(assetManager).dutchPay(_nftContractAddress, _tokenId, msg.sender, lender, price, dutchAuctions[_nftContractAddress][_tokenId].endPrice);
    delete dutchAuctions[_nftContractAddress][_tokenId];
    delete items[_nftContractAddress][_tokenId];
}

Recommendation:

We advise the SalvorLending::makeBidForDutchAuction function to be accessible solely when the auction is active rather than when it has started, ensuring that bids cannot be made at the same time extensions and clearances can occur.

HKskn commented 7 months ago

Omniscia: SLG-03M: Understood, we will make sure to reflect this in the audit report. We can either remove the finding entirely, or add an alleviation chapter that states it was incorrect. Whichever you opt for is fine by us, and we value transparency!