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

Misshandling of ETH, wrong address to send funds to

farnad

Root + Impact

Description

  • Due to inconsistency, in BidBeastsNFTMarket::withdrawAllFailedCredits the funds are sent to the wrong address.

  • It is checked to see if the _receiver address has any failed credits. But then, mistakenly the fund is sent 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:

  • It is certain and happens every signle time.

Impact:

  • It is devastating. It drains users funds to the attacker account.

Proof of Concept

Add the following function to the test file and run it.

function test_stealBidderFunds() public {
// Arrange
_mintNFT();
_listNFT();
vm.deal(address(rejector), 100 ether);
vm.startPrank(address(rejector));
market.placeBid{value: MIN_PRICE + 1}(TOKEN_ID);
vm.stopPrank();
vm.startPrank(BIDDER_1);
market.placeBid{value: BUY_NOW_PRICE}(TOKEN_ID);
vm.stopPrank();
//Act
vm.prank(BIDDER_2);
uint256 balanceBefore = BIDDER_2.balance;
market.withdrawAllFailedCredits(address(rejector));
uint256 balanceAfter = BIDDER_2.balance;
//Assert
assert(balanceAfter > balanceBefore);
}
​

Recommended Mitigation

Replace msg.sender cases with _receiver to send the funds to the correct receiver.

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
// @audit Misshandling of ETH - msg.sender is used instead of _receiver
// This allows anyone to withdraw someone else's credits to their own address.
// Should be:
// failedTransferCredits[_receiver] = 0; // and
// (bool success, ) = payable(_receiver).call{value: amount}("");
​
- failedTransferCredits[msg.sender] = 0;
​
- (bool success, ) = payable(msg.sender).call{value: amount}("");
+ failedTransferCredits[_receiver] = 0;
​
+ (bool success, ) = payable(_receiver).call{value: amount}("");
require(success, "Withdraw failed");
}
​
Updates

Lead Judging Commences

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