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

Unauthorized withdrawal & persistent drain via withdrawAllFailedCredits (wrong mapping key + missing authorization)

shashankwcw

withdrawAllFailedCredits reads the amount from failedTransferCredits[_receiver] but sets failedTransferCredits[msg.sender] = 0 and sends ETH to msg.sender. There is no authorization check that _receiver == msg.sender. The victim’s mapping entry is never cleared, allowing repeated withdrawals of the same amount. This is a logic/indexing bug in the withdraw function combined with missing ownership checks.

Description

  • Normal behavior: Users who have ETH credited to failedTransferCredits (due to previous failed payouts) should be able to withdraw only their own credited balance. A correct implementation reads the caller’s credited amount, zeroes that balance (effects), and then transfers the funds to the caller (interaction), protecting against reentrancy and emitting an event for auditability.

  • Specific issue: The function withdrawAllFailedCredits(address _receiver) reads the amount from failedTransferCredits[_receiver] but zeroes failedTransferCredits[msg.sender] and sends the ETH to msg.sender, without any authorization check that _receiver == msg.sender. This logic/indexing error lets any attacker request withdrawals for any victim and receive the victim’s credited amount into their own account while leaving the victim’s mapping unchanged — enabling repeated withdrawals of the same funds and potential full contract drain.


Vulnerable piece of code

@> 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:

  • When any address has a nonzero failedTransferCredits balance, any external account can call withdrawAllFailedCredits(address) with that address and receive the credited ETH while the victim’s mapping remains unchanged.

  • Because the mapping is public, automated bots or MEV operators can scan all credited balances and repeatedly exploit the function, making exploitation highly probable.

Impact:

  • Direct and repeated theft of credited funds: attackers can repeatedly withdraw another user’s credited ETH, causing immediate financial loss to users and depletion of contract funds.

  • Severe operational and reputational damage: legitimate withdrawals fail or are contested, marketplace operations are disrupted, and user trust and platform reputation suffer.

Proof of Concept

  • After successful execution of test the attaker's balance will be 4000000000000000000
    because the rejector's address made a bid for 2 ethers

function test_anyonecancallwithdraw() public {
_mintNFT();
_listNFT();
vm.startPrank(address(rejector));
market.placeBid{value: 2 ether}(TOKEN_ID);
vm.stopPrank();
vm.startPrank(BIDDER_2);
market.placeBid{value: 3 ether}(TOKEN_ID);
vm.stopPrank();
console.log("balance of rejector after withdraw", address(rejector).balance);
console.log("balance of attacker before withdraw", ATTACKER.balance);
vm.startPrank(ATTACKER);
market.withdrawAllFailedCredits(address(rejector));
market.withdrawAllFailedCredits(address(rejector));
vm.stopPrank();
console.log("balance of attacker after withdraw", ATTACKER.balance);
}
​
​
​

Recommended Mitigation

  1. The mapping failedTransferCredits will check the amount in the context of msg.sender so if an address doesn't have the balance then the function will revert

  2. The mapping failedTransferCredits will reduce in the context of msg.sender

  3. I also recommend adding nonReentrant
    because in future this contract might become vulnerable to cross function or cross contract reentrancy

function withdrawAllFailedCredits() external nonReentrant {
uint256 amount = failedTransferCredits[msg.sender];
require(amount > 0, "No failed 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 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!