Root + Impact

Description

• The normal behavior of withdrawAllFailedCredits should allow a user to withdraw their own failed transfer credits that accumulated when a direct transfer to them failed.

• The specific issue is that the function uses an arbitrary _receiver address parameter to read the credit amount, but then sends this amount to msg.sender while zeroing out msg.sender's credits. This allows any user to steal another user's credits.

Risk

Likelihood: High

• Any user with knowledge of another user's address who has accumulated failed transfer credits can extract those credits.

• The vulnerability is trivially exploitable with a single transaction and no special conditions required beyond knowing an address with credits.

Impact: Critical

• Complete theft of accumulated failed transfer credits from any user.

• Double-spending of contract funds, potentially leading to contract insolvency when legitimate users attempt to withdraw their credits.

Proof of Concept

The vulnerability in withdrawAllFailedCredits allows any user to steal another user's failed transfer credits. Here's a detailed explanation of how the exploit works:
The PoC demonstrates a real-world attack scenario where an attacker creates a contract to steal another user's failed transfer credits:

1. Setup: The attack assumes Alice has accumulated failed transfer credits (e.g., 10 ETH) in the marketplace contract.

2. Attack Vector: The vulnerability stems from the parameter confusion in the withdrawAllFailedCredits function, where the function:

   • Takes an arbitrary address as input (_receiver)

   • Reads the credits amount from that address's balance

   • But zeroes out the caller's balance

   • And sends the funds to the caller

3. Execution Flow:

   • Attacker deploys the Exploit contract targeting the marketplace

   • Calls attack(alice_address) which executes withdrawAllFailedCredits(alice_address)

   • The function reads Alice's credit balance (10 ETH)

   • Zeroes the Exploit contract's credits (which were likely 0 anyway)

   • Transfers 10 ETH to the Exploit contract

4. Result:

   • The attacker now possesses Alice's 10 ETH

   • Alice's credits remain intact in storage (they were never zeroed)

   • When Alice attempts to withdraw her credits later, the contract will try to pay her again

   • This creates a double-spending situation that could drain the contract

   • The exploit requires no special conditions or permissions, making it extremely easy to execute against any user with accumulated failed transfer credits.

Recommended Mitigation

The proposed mitigation fixes the fundamental design flaw in the withdrawAllFailedCredits function by ensuring users can only withdraw their own failed transfer credits:

1. Remove the Parameter:

   • Eliminate the _receiver parameter entirely, which removes the ability to specify another user's address

   • This ensures users can only interact with their own credits

2. Self-Referential Logic:

   • Use msg.sender consistently throughout the function for both:

   • Reading the credit amount

   • Zeroing the appropriate balance

   • Sending the funds to the rightful owner

3. Function Renaming:

   • Change the function name to withdrawFailedCredits to better reflect its purpose and avoid confusion

4. Benefits of the Fix:

   • Eliminates the possibility of credit theft

   • Ensures proper accounting of funds

   • Prevents potential contract insolvency from double-spending

   • Maintains the original functionality of allowing users to withdraw their failed credits

   • Follows the principle of least privilege by restricting users to their own assets