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

Stealing of people's failed credit

gridgirl

Root + Impact

Description

  • Normal Behaviour: Users withdraw their own failed-transfer credit(pull payments) - the contract reads the stored credit for the caller and sends it to that caller.

  • Issue: The function reads <failedTransferCredits[_receiver] but zeroes and pays <msg.sender>. This allows any caller to request withdrawl of another address credits and receive them.

// Root cause in the codebase with @> marks to highlight the relevant section
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:

  • Reason 1 A caller can repeatedly call withdrawAllFailedCredits(victim) when victim has credits (occurs whenever any failed payouts exist for victim).

  • Reason 2: Automated bots or any scripts will easily abuse this because only one straightforward call is required.

Impact:

  • Impact 1: Permanent loss of funds for victims (credits moved to attacker)

  • Impact 2: Severr trust/legal consequences- marketplace users lose funds and reputation of the platform is destroyed

Proof of Concept

// PoC: attacker steals victim's failed-transfer credits by calling withdrawAllFailedCredits(victim).
// Steps (Ethers / Foundry style pseudo-code):
​
// 1) Assume victim has a failed credit of 1 ether recorded:
// failedTransferCredits[victim] == 1 ether
//
// 2) Attacker calls the vulnerable function specifying the victim's address:
await marketplace.connect(attacker).withdrawAllFailedCredits(victim.address);
​
// 3) Internal behavior (matching vulnerable code):
// uint256 amount = failedTransferCredits[_receiver]; // reads victim's 1 ETH
// require(amount > 0, "No credits to withdraw");
// failedTransferCredits[msg.sender] = 0; // clears attacker's slot (likely 0)
// (bool success, ) = payable(msg.sender).call{value: amount}(""); // pays attacker
​
// Result:
// - Attacker receives 1 ETH.
// - Mapping slot failedTransferCredits[victim] remains unchanged (still shows 1 ETH),
// or bookkeeping is inconsistent. Victim lost funds; attacker profited.
//
// Explanation:
// - The contract reads credits using the caller-specified `_receiver` but writes and pays `msg.sender`.
// - Because of that mix-up, specifying any address that has credits causes the contract to send those credits to the caller.
// - This requires only one external call and will succeed whenever a non-zero credit exists for the supplied victim address.
​
​

Recommended Mitigation

- remove this code
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");
}
//looks up how much ETH is credited to the _receiver address
//It requires that the amount is > 0.
//It then clears the balance for msg.sender, not for _receiver and sends the victim’s credits (amount) to msg.sender
​
+ add this code
​
/// @notice Emitted when a user successfully withdraws their credited funds.
event FailedCreditWithdrawn(address indexed who, uint256 amount);
​
/// @notice Withdraw caller's failed-transfer credits (pull pattern).
/// @dev Uses Checks-Effects-Interactions and nonReentrant to prevent reentrancy.
​
function withdrawFailedCredits() external nonReentrant {
uint256 amount = failedTransferCredits[msg.sender];
require(amount > 0, "No credits to withdraw");
​
// Effects: clear the mapping before external call
failedTransferCredits[msg.sender] = 0;
​
// Interaction: attempt to send funds to the caller
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
​
emit FailedCreditWithdrawn(msg.sender, amount);
}
//The fixed code removes the ability to specify someone else’s address, makes the mapping authoritative, clears state before sending funds, and blocks reentrancy — together these changes eliminate the original theft vector and make withdrawals safe and auditable.
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 10 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!