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

Critical Vulnerability in withdrawAllFailedCredits Allows Fund Drainage

sagarchhetri

Root + Impact

Description

  • Normal Behavior: The withdrawAllFailedCredits function is intended to allow a user to withdraw their own funds that were credited to them after a failed refund or payment.


  • The Issue: The function contains two critical flaws. First, it lacks access control, allowing any attacker (msg.sender) to withdraw the credits of another user (_receiver). Second, it incorrectly resets the attacker's balance to zero instead of the victim's, which allows the victim's funds to be drained repeatedly by multiple attackers.

// 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");
@> failedTransferCredits[msg.sender] = 0;
@> (bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • The vulnerability will occur whenever a user's refund fails (creating a credit balance) and an attacker identifies that user's address. The exploit requires no special conditions or permissions.


Impact:

  • This flaw leads to the direct and permanent loss of user funds. Because the victim's balance is never decremented, the attack can be repeated until the entire credit balance is stolen.


Proof of Concept

Explanation: The Proof of Concept test first creates the vulnerable condition by using a special ToggleRefundBidder contract whose refund is designed to fail, causing funds to be credited. It then demonstrates that the fix is effective by showing that an attacker's attempt to withdraw the victim's credits is now correctly reverted with a "Not receiver" error.

function test_exploit_withdrawAllFailedCredits_drainsVictim() public {
// Mint and list NFT by normal SELLER
_mintNFT();
_listNFT();
​
// Deploy a bidder contract that rejects refunds
ToggleRefundBidder victim = new ToggleRefundBidder();
​
// Victim places the first bid at min price
vm.deal(address(victim), STARTING_BALANCE);
vm.prank(address(victim));
victim.bid{value: MIN_PRICE}(address(market), TOKEN_ID);
​
// Another user outbids, triggering refund to victim which will fail and be credited
uint256 secondBidAmount = (MIN_PRICE * 105) / 100;
vm.prank(BIDDER_1);
market.placeBid{value: secondBidAmount}(TOKEN_ID);
uint256 victimCreditsBefore = market.failedTransferCredits(address(victim));
assertGt(victimCreditsBefore, 0, "Expected failed credits for victim bidder");
​
// Attacker attempts to drain victim's credits and is reverted by the fix
vm.prank(BIDDER_2);
vm.expectRevert("Not receiver");
market.withdrawAllFailedCredits(address(victim));
}

Reference Files

  • Vulnerable Code: https://github.com/CodeHawks-Contests/2025-09-bid-beasts/blob/main/src/BidBeastsNFTMarketPlace.sol#L238-L247


  • Proof of Concept Test: https://github.com/Sagarchhetri83/2025-09-bid-beasts/blob/main/test/BidBeastsMarketPlaceTest.t.sol#L167-L205


Recommended Mitigation

Explanation: The fix remediates the vulnerability by adding a require(msg.sender == _receiver) check, which ensures only the owner of the credits can initiate a withdrawal. It also corrects the logic to zero out the _receiver's balance instead of the msg.sender's, preventing the replay attack.

- 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 {
+ require(msg.sender == _receiver, "Not receiver");
+ uint256 amount = failedTransferCredits[_receiver];
+ require(amount > 0, "No credits to withdraw");
+
+ failedTransferCredits[_receiver] = 0;
+
+ (bool success, ) = payable(_receiver).call{value: amount}("");
+ require(success, "Withdraw failed");
+ }
Updates

Lead Judging Commences

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

Give us feedback!