Skip to content

Reentrancy Vulnerability in Remaining Auction Contracts #35

@rohans02

Description

@rohans02

Summary

EnglishAuction, AllPayAuction, and all Dutch auction variants are vulnerable to reentrancy attacks due to external calls before state updates without protection.

Note: Extends the VickreyAuction reentrancy issue to the remaining 5 auction contracts.


Affected Contracts

  • EnglishAuction.sol
  • AllPayAuction.sol
  • LinearReverseDutchAuction.sol
  • ExponentialReverseDutchAuction.sol
  • LogarithmicReverseDutchAuction.sol

Problem

External calls happen before state updates, violating Checks-Effects-Interactions pattern:

1. NFT Transfers in claim()

IERC721(token).safeTransferFrom(address(this), winner, tokenId);
// onERC721Received callback can reenter before:
auction.isClaimed = true;

2. ERC20 Transfers in withdraw() and bid()

sendERC20(biddingToken, recipient, amount);
// Callback possible before state finalization

Impact

Severity: High

  • Multiple claims via NFT callback reentrancy
  • State corruption during bidding/withdrawals
  • Auction manipulation through external calls

Solution

Add OpenZeppelin ReentrancyGuard:

abstract contract Auction is IERC721Receiver, ReentrancyGuard {
    // All auctions inherit protection
}

function claim(uint256 auctionId) external nonReentrant { 
    // Protected
}

Protection for: claim(), withdraw(), bid() functions


Testing

Will include:

  • Attack scenario demonstrations
  • ReentrancyGuard effectiveness tests
  • Malicious contract mocks
  • Verification of existing functionality

I'd like to work on this as an extension of the VickreyAuction reentrancy fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions