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

Missing Zero Address Check for `_receiver` in `BidBeastsNFTMarket::withdrawAllFailedCredits`

0xzhd

Root + Impact

  • Root Cause: The withdrawAllFailedCredits function lacks a validation to ensure the _receiver address is not the zero address (address(0)). If the failedTransferCredits mapping contains a non-zero balance for address(0)—potentially due to a bug in another function like _payout—any user can call withdrawAllFailedCredits(address(0)) to withdraw those funds. The absence of this check, combined with the existing issue of clearing failedTransferCredits[msg.sender] instead of _receiver, enables unauthorized access.

  • Impact: This vulnerability allows any user to steal funds credited to the zero address, leading to potential financial loss. It exacerbates the related mapping update bug, enabling repeated exploitation without affecting the zero address balance, thereby undermining the contract’s security and user trust.

Description:

The withdrawAllFailedCredits function in the BidBeastsNFTMarket contract does not verify that the _receiver address is not the zero address (0x0). If the failedTransferCredits mapping contains a non-zero balance for address(0) (e.g., due to a bug in another function like _payout), any user can call withdrawAllFailedCredits(address(0)) to withdraw those funds. This allows unauthorized access to funds credited to the zero address, which should not be accessible.

function withdrawAllFailedCredits(address _receiver) external {
@> // No zero address check here
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: Medium. The vulnerability depends on the failedTransferCredits[address(0)] having a non-zero balance, which may occur due to bugs in payout logic or external interactions. The likelihood increases in complex contracts with multiple fund-handling functions.

  • Impact: Medium. Unauthorized withdrawals from the zero address result in financial loss, though the impact is limited to the amount credited there. Combined with the mapping bug, it amplifies trust and security concerns, though it does not affect all users directly.

Proof of Concept:

This test demonstrates the vulnerability by showing that an attacker can withdraw funds credited to the zero address (address(0)) without any authorization. The test sets a balance of 2 ether for address(0) in the failedTransferCredits mapping and verifies that an attacker can steal these funds.

Add the following test to the BidBeastsNFTMarketTest.t.sol test file to reproduce the issue:

Code
function testExploitZeroAddressWithdrawal() public {
address attacker = makeAddr("Attacker");
​
// Fund the contract with 100 ether
vm.deal(address(market), 100 ether);
​
// Confirm the mapping storage slot index for failedTransferCredits (slot 5)
uint256 mappingSlot = 5;
​
// Set 2 ether credits for the zero address
bytes32 zeroKey = keccak256(abi.encode(address(0), mappingSlot));
vm.store(address(market), zeroKey, bytes32(uint256(2 ether)));
​
// Sanity check: Verify the zero address has 2 ether credits
assertEq(market.failedTransferCredits(address(0)), 2 ether, "Zero address should have 2 ether credits");
​
// Record attacker's initial balance
uint256 attackerBalanceBeforeAttack = attacker.balance;
uint256 initialZeroBalance = market.failedTransferCredits(address(0)); // 2 ether
​
// Attacker calls withdrawAllFailedCredits with zero address
vm.prank(attacker);
market.withdrawAllFailedCredits(address(0));
​
// Record attacker's balance after attack
uint256 attackerBalanceAfterAttack = attacker.balance;
​
// Log balances for debugging
console.log("Attacker's balance before attack:", attackerBalanceBeforeAttack);
console.log("Attacker's balance after attack:", attackerBalanceAfterAttack);
​
// Check that attacker received 2 ether
assertEq(attackerBalanceAfterAttack, attackerBalanceBeforeAttack + 2 ether, "Attacker should receive 2 ether");
​
// Zero address credits remain unchanged (due to the bug resetting msg.sender's credits)
assertEq(market.failedTransferCredits(address(0)), initialZeroBalance, "Zero address credits should remain unchanged");
​
// Attacker's own credits are reset to 0
assertEq(market.failedTransferCredits(attacker), 0, "Attacker credits should be reset to 0");
​
// Logs:
// Attacker's balance before attack: 0
// Attacker's balance after attack: 2000000000000000000
}

Explanation

  • Setup: The zero address is credited with 2 ether in failedTransferCredits[address(0)] via storage manipulation.

  • Attack: The attacker calls withdrawAllFailedCredits(address(0)), receiving 2 ether while resetting their own (zero) balance.

  • Result: The zero address’s credits remain 2 ether (unaffected due to the mapping bug), confirming the vulnerability.

Recommended Mitigation:

To prevent unauthorized withdrawals from the zero address, add a check to ensure _receiver is not address(0). Additionally, consider addressing the related issue of incorrect mapping updates (resetting msg.sender instead of _receiver) to fully secure the function.

Example Fix:

+ error ReceiverCannotBeZeroAddress();
​
function withdrawAllFailedCredits(address _receiver) external {
+ if (_receiver == address(0)) {
+ revert ReceiverCannotBeZeroAddress();
+ }
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.