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

[M-03] Reentrancy risk in `withdrawAllFailedCredits`

agilegypsy

Root + Impact

Description

The withdrawAllFailedCredits function should follow the check-effect-interact pattern to prevent reentrancy attacks. This ensures atomic operations and prevents malicious contracts from manipulating state during external calls.

The specific issue is that the function makes an external ETH transfer call before updating the contract state, creating a potential reentrancy vulnerability where malicious contracts could re-enter the function during the external call while state is inconsistent.

// Root cause in the codebase
function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
failedTransferCredits[_receiver] = 0; // @> State update after external call (wrong order)
(bool success, ) = payable(msg.sender).call{value: amount}(""); // @> External call before state update
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • Reentrancy attacks require malicious contracts to be deployed

  • Any user can trigger the reentrancy pattern by calling with malicious contracts

  • Requires specific conditions but is technically exploitable

Impact:

  • Potential state manipulation during external calls

  • Violation of security best practices (check-effect-interact)

  • Could lead to more complex attack vectors when combined with other vulnerabilities

  • Risk of double-spending or state corruption scenarios

Proof of Concept

Reentrancy window exists during external ETH transfer when state is inconsistent:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {BidBeastsNFTMarketPlace} from "../src/BidBeastsNFTMarketPlace.sol";
​
contract ReentrancyTest is Test {
BidBeastsNFTMarketPlace marketplace;
function testReentrancyWindow() public {
// During external call, state is not yet updated
// This creates a reentrancy window
marketplace.withdrawAllFailedCredits(address(this));
}
}

Recommended Mitigation

Update state before external calls to prevent reentrancy.

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
+ failedTransferCredits[_receiver] = 0; // Update state first
(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!