sherlock-audit / 2024-02-radicalxchange-judging

3 stars 1 forks source link

AMOW - Protocol funds can be stolen through malicious bidding #23

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

AMOW

high

Protocol funds can be stolen through malicious bidding

Summary

Malicious actor can inflate their availableCollateral and withdraw funds on the back of the protocol

Vulnerability Detail

Imagine the following scenario: Malicious actors Bob and Alice, Bob is the current owner of an auctioned NFT. Bob transfers NFT ownership to Alice Alice bids and passes this check Alice pays only feeAmount as collateral (which is a fraction of bidAmount) Alice transfers the NFT back to Bob Alice wins the auction while Bob is NFT owner and the following logic executes

else if (
            l.highestBids[tokenId][currentAuctionRound].bidder != oldBidder // makes sure highestBid belongs to a user who is not ownerOf the NFT
        ) {
            // Transfer bid to previous bidder's collateral
            l.availableCollateral[oldBidder] += l.highestBids[tokenId][currentAuctionRound].bidAmount; // bidAmount is transferred to Bob's availableCollateral 
            l.highestBids[tokenId][currentAuctionRound].collateralAmount = 0;
            l.bids[tokenId][currentAuctionRound][l.highestBids[tokenId][currentAuctionRound].bidder].collateralAmount = 0;
        }

Bob's availableCollateral is incremented by bidAmount despite Alice submitting only a fraction of it as collateral

Impact

Loss of funds

Code Snippet

if (IStewardLicense(address(this)).exists(tokenId)) {
            currentBidder = IStewardLicense(address(this)).ownerOf(tokenId);
        } else {
            currentBidder = l.initialBidder;
        }

        if (bidder == currentBidder) {
            // If current bidder, collateral is entire fee amount
            feeAmount = totalCollateralAmount;
        }
        ...

else if (
            l.highestBids[tokenId][currentAuctionRound].bidder != oldBidder // makes sure highestBid belongs to a user who is not ownerOf the NFT
        ) {
            // Transfer bid to previous bidder's collateral
            l.availableCollateral[oldBidder] += l.highestBids[tokenId][currentAuctionRound].bidAmount; // bidAmount is transferred to Bob's availableCollateral 
            l.highestBids[tokenId][currentAuctionRound].collateralAmount = 0;
            l.bids[tokenId][currentAuctionRound][l.highestBids[tokenId][currentAuctionRound].bidder].collateralAmount = 0;
        }

Tool used

Manual Review

Recommendation

Entire function logic is flawed, needs revision

sherlock-admin2 commented 8 months ago

Escalate

(on behalf of watson) valid issue.

You've deleted an escalation for this issue.

gravenp commented 8 months ago

I believe this is a duplicate of https://github.com/sherlock-audit/2024-02-radicalxchange-judging/issues/9 @Hash01011122