Short summary

A mismatch between the mapping key read and the mapping key cleared in withdrawAllFailedCredits allows any attacker to withdraw another address’ failedTransferCredits repeatedly, draining contract funds.

Description

• Normal behaviour:

  When a low-level transfer to a recipient fails, the contract should record that recipient’s credit in failedTransferCredits[recipient]. Only the credited recipient (or a clearly authorized party) should be able to withdraw those funds, and the withdrawn credit must be removed (atomically) from the mapping.

• Observed behaviour (issue):

  withdrawAllFailedCredits(address receiver) reads amount = failedTransferCredits[receiver] but then clears failedTransferCredits[msg.sender] (or otherwise does not atomically clear the same mapping entry), and pays msg.sender. Because the mapping entry for receiver is left unchanged, an attacker may repeatedly call withdrawAllFailedCredits(receiver) and receive the same amount each time — until the contract’s ETH is exhausted. This results in direct theft from the contract.

Root cause

Key mismatch between read and clear/write operations:

• read uses receiver as the key,

• clear/write uses msg.sender (or some other key),

• pay goes to msg.sender.

This is a logic bug (indexing bug) that leaves the victim’s credit record intact while paying the attacker.

Risk

Likelihood:

• The attack requires only the existence of a nonzero failedTransferCredits[Victim] entry and the ability to call withdrawAllFailedCredits(receiver).

• No privileges are required — any EOA can initiate the withdraw call — so the attack is trivial to trigger.

• An attacker can bootstrap the exploit themselves — e.g., place bids from a contract that deliberately rejects Ether (so refunds fail and are credited), then repeatedly call withdrawAllFailedCredits with that contract as the receiver to siphon funds, without waiting for other users.

Impact:

• An attacker can repeatedly withdraw the same credited amount while the mapping entry remains, drawing repeated transfers from the contract.

• The contract balance is the limiting factor; attacker can extract floor(contract_balance / credit_amount) * credit_amount. If credits and contract balance are large, this is direct theft of funds (complete or partial draining).

• Financial loss to users and protocol (seller refunds stolen, funds missing), reputational damage, potential legal exposure.

Proof of Concept

Place the following into BidBeastsMarketPlaceTest.t.sol.

This PoC demonstrates both theft and repeatability.

Recommended Mitigation

Allow only the credited address to withdraw its own credits. This is the best-practice unless you have a strong reason to let third parties withdraw on behalf of someone.

Why this works: read/clear/pay all use the same key (receiver), and only the intended owner can trigger the withdraw.

Additional protections (apply in all fixes)

• Add nonReentrant (OpenZeppelin ReentrancyGuard) to withdrawAllFailedCredits and other money-moving functions: placeBid, takeHighestBid, settleAuction, withdrawFee.