code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Bid's array can be overloaded with dust bids to break AuctionDemo funcionality. #2013

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L69 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L136 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L110 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L90

Vulnerability details

Description

Bids to the auction can be created using any msg.value via participateToAuction method and are stored in an array within the mapping auctionInfoData. However, all important methods (claimAuction, returnHighestBid, returnHighestBidder, participateToAuction) loops through the array, which can allow an attacker to fulfill it with numerous dust valueless bids to cause these functions to revert due to out of gas errors. In the worst case, where he can't overload the array enough, he will still impact other users since they will be potentially spending more gas (more money) than expected.

Proof of Concept

  1. An auction has just started.

  2. Attacker calls participateToAuction as much as he can:

    function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
        auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
        auctionInfoData[_tokenid].push(newBid);
    }
    • Every single call executes auctionInfoData[_tokenid].push(newBid);, increasing the length of the bid's array.
  3. The function requires msg.value to be higher than the highestBid. However, a new auction won't have any bid, so the attacker just needs to increment 1 wei in each participateToAuction call.

  4. After he called it enough times, essential functions like participateToAuction will try to loop through auctionInfoData[_tokenid], but will spend enough gas to revert or make the transaction a lot more expensive than it should, which impacts any user of the protocol.

Impact

Attacker can interrupt an auction or make every function call a lot more expensive.

  1. Likelihood: Low. Attacker needs to do in starts of auction and a non-dust bid can revert the attack, since new bids need to be the highest price of the array.
  2. Impact: High. Attacker can make every user overspend gas values and potentially revert the whole protocol, which doesn't have any emergency withdraw function.
    • Risk: Medium (Low Likelihood with High Impact)

Tools Used

Manual Review

Recommended Mitigation Steps

Define a minimum value check in participateAuction. After that, any attack like this will have a high cost.

Assessed type

Loop

c4-pre-sort commented 10 months ago

141345 marked the issue as duplicate of #1952

c4-judge commented 10 months ago

alex-ppg marked the issue as duplicate of #2038

c4-judge commented 9 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope

c4-judge commented 9 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope