Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Failed-credits theft & infinite drain: withdrawAllFailedCredits(address _receiver) lets anyone withdraw others’ credits; victim balance isn’t decremented

berkep1914

Root + Impact

Description

  • Normal behavior: When a direct ETH transfer fails (e.g., refund to previous bidder or seller proceeds), the contract should credit the intended recipient and later allow only that same recipient to withdraw their own credits. The withdrawal must decrement the credited balance to prevent repeated drains.

  • Issue: withdrawAllFailedCredits(address _receiver) reads the amount from failedTransferCredits[_receiver], but then zeros failedTransferCredits[msg.sender] (different key) and sends the funds to msg.sender. The victim’s credit under _receiver is never reduced, enabling any address to steal another user’s credits and repeat the withdrawal indefinitely (until the contract’s ETH balance is exhausted). Reentrancy isn’t required, but would amplify the drain.

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
// @> BUG: zeroing the wrong slot (msg.sender instead of _receiver)
failedTransferCredits[msg.sender] = 0;
​
// @> BUG: sending to msg.sender allows anyone to steal others' credits
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}
​

Risk

Likelihood:

  • Occurs whenever any failed credits exist, e.g., a previous highest bidder or seller is a contract without a payable receive/fallback or one that reverts on ETH receipt (easy to engineer by bidding via such a contract).

Attackers simply call withdrawAllFailedCredits(victim) to siphon the victim’s credits to themselves; since the victim’s slot isn’t decremented, the attacker can repeatedly call and drain the contract’s ETH balance.

Impact:

  • Theft of funds: Credits belonging to other users are paid to the attacker.

Infinite drain: Because the victim’s credit is never decremented, repeated calls (or reentrancy) can drain all ETH held by the marketplace.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {BidBeastsNFTMarket} from "../src/BidBeastsNFTMarketPlace.sol";
​
// A bidder that causes refunds to fail -> creates failedTransferCredits for itself
contract RejectEther {
// No payable receive/fallback -> any ETH transfer to this contract fails
}
​
// Attacker contract that steals someone else's failed credits
contract StealCreditsPoC {
BidBeastsNFTMarket market;
​
constructor(BidBeastsNFTMarket _market) { market = _market; }
​
// Steal credits recorded under `victim` into this contract (msg.sender = this)
function steal(address victim) external {
// Reads amount from failedTransferCredits[victim],
// zeroes failedTransferCredits[msg.sender],
// and transfers to msg.sender (this contract).
market.withdrawAllFailedCredits(victim);
}
​
// Demonstrate repeated drain: call steal(victim) multiple times
// because victim's credit is never decremented by the buggy function.
}
​

Attack flow example (end-to-end):

  1. An address victim (e.g., a previous highest bidder using RejectEther) accumulates credits via _payout(victim, amount) → transfer fails → failedTransferCredits[victim] += amount.

  2. Attacker calls withdrawAllFailedCredits(victim) once → receives amount to attacker, while failedTransferCredits[victim] remains unchanged.

  3. Attacker repeats the call to siphon the same amount again, draining the contract’s ETH balance.


Recommended Mitigation

Additional hardening:

  • Add nonReentrant to the withdrawal function to block reentrancy amplification.

  • Keep CEI ordering (zero balance before external call).

  • Consider emitting an event FailedCreditsWithdrawn(msg.sender, amount) for auditability.

- function withdrawAllFailedCredits(address _receiver) external {
- uint256 amount = failedTransferCredits[_receiver];
- require(amount > 0, "No credits to withdraw");
-
- failedTransferCredits[msg.sender] = 0;
-
- (bool success, ) = payable(msg.sender).call{value: amount}("");
- require(success, "Withdraw failed");
- }
+ function withdrawAllFailedCredits(address _receiver) external {
+ // Option A: restrict to self-withdrawals only
+ require(_receiver == msg.sender, "Only withdraw own credits");
+ uint256 amount = failedTransferCredits[msg.sender];
+ require(amount > 0, "No credits to withdraw");
+ failedTransferCredits[msg.sender] = 0; // zero the correct slot (effects)
+ (bool success, ) = payable(msg.sender).call{value: amount}(""); // interactions
+ require(success, "Withdraw failed");
+ }
+
+ // Alternatively (Option B): remove the parameter entirely to avoid misuse
+ // function withdrawFailedCredits() external nonReentrant {
+ // uint256 amount = failedTransferCredits[msg.sender];
+ // require(amount > 0, "No credits to withdraw");
+ // failedTransferCredits[msg.sender] = 0;
+ // (bool success, ) = payable(msg.sender).call{value: amount}("");
+ // require(success, "Withdraw failed");
+ // }
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 25 days ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeast Marketplace: Unrestricted FailedCredits Withdrawal

withdrawAllFailedCredits allows any user to withdraw another account’s failed transfer credits due to improper use of msg.sender instead of _receiver for balance reset and transfer.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.