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

Incorrect credit withdrawal logic enables theft & double-withdrawals

emfarih

Root + Impact

Description

  • The normal behavior is that a user should only be able to withdraw their own accumulated failedTransferCredits, and the contract should decrease that user’s credit balance atomically upon payout.

  • The issue is in withdrawAllFailedCredits(address _receiver): the function reads credits from _receiver, but zeros out msg.sender’s bucket and then pays msg.sender. The credited bucket for _receiver is never decremented, enabling repeated withdrawals by anyone (not just _receiver) as long as the contract holds ETH.

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
@> failedTransferCredits[msg.sender] = 0; // zeros the WRONG bucket
@>
@> (bool success, ) = payable(msg.sender).call{value: amount}(""); // pays msg.sender, not _receiver
require(success, "Withdraw failed");
}
​

Risk

Likelihood:

  • Occurs whenever failedTransferCredits[_receiver] becomes non-zero (e.g., a refund/payout failed and was credited).

  • Any caller can trigger the withdrawal by passing _receiver with a positive balance, regardless of caller identity.

Impact:

  • Unauthorized withdrawal: An attacker calls with _receiver = victim address → receives the victim’s credits to attacker’s address.

  • Double-withdrawal: Since _receiver’s bucket is not decremented, the same credits can be withdrawn repeatedly until the contract ETH is depleted.

Proof of Concept

// Assume failedTransferCredits[victim] = X (>0), e.g., from a prior failed payout.
​
// Attacker calls:
market.withdrawAllFailedCredits(victim);
​
// Effects due to the bug:
// 1) amount = failedTransferCredits[victim] (X)
// 2) failedTransferCredits[msg.sender] = 0 (attacker's bucket zeroed; victim's untouched)
// 3) Sends X ETH to attacker (msg.sender)
// 4) failedTransferCredits[victim] still equals X
​
// Attacker repeats the same call multiple times to drain contract ETH.
// Victim later tries to withdraw but credits may have already been stolen.
​

The bug comes from mixing _receiver (victim’s slot) with msg.sender (attacker). The contract always pays msg.sender but never reduces _receiver’s balance, creating unlimited withdrawal opportunities for attackers.

Recommended Mitigation

- 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");
- }
+ // Restrict withdrawals to the caller's own credits and zero out the correct bucket.
+ // Consider adding nonReentrant for extra safety.
+ function withdrawAllFailedCredits() external {
+ 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");
+ }
​

The fix removes the _receiver parameter entirely. Users can now only withdraw their own credits (msg.sender). The correct bucket is read and then cleared before funds are transferred. This prevents attackers from targeting other users’ balances and stops infinite re-withdrawals.

Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month 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.