Root + Impact

Description

• The withdrawAllFailedCredits(address _receiver) function reads failedTransferCredits[_receiver] into amount, but then clears failedTransferCredits[msg.sender] and sends amount to msg.sender.

• This allows any caller to steal someone else’s credited balance by passing that victim’s address as _receiver. The victim’s mapping entry is not cleared, so the attacker can repeatedly drain or block withdrawals.

Risk

Likelihood:

• Wrong account cleared: The function reads the victim’s credit but zeros out the caller’s mapping entry (failedTransferCredits[msg.sender] = 0) instead of zeroing failedTransferCredits[_receiver].

• Improper authorization: There is no check that msg.sender is allowed to withdraw _receiver’s credits (no msg.sender == _receiver or owner-only guard).

• Checks-Effects-Interactions order broken: The effects (clearing the mapping) are applied to the wrong account; interactions (external call) occur after a wrong effect, violating intended checks-effects-interactions semantics.

  
  

  Because of (1) and (2), the function effectively gives any caller the ability to request the amount belonging to any other address and be paid that amount.

  
  

Impact:

• Immediate theft of credited ETH from the contract by any attacker. Because the victim’s stored credit is not cleared, the attack can be repeated or used to prevent the legitimate owner from ever withdrawing.

  
  

Proof of Concept

• When the rejector reverts upon receiving a refund, _payout falls back to recording the value in failedTransferCredits[rejector] (this is the contract’s “fallback credit” mechanism).

• Then the attacker calls withdrawAllFailedCredits(rejector). This function reads the credits of _receiver, but resets failedTransferCredits[msg.sender] to zero, and finally transfers the amount to msg.sender (the attacker). As a result, the attacker takes the credits that belong to rejector, while the mapping entry for rejector remains uncleared → enabling repeated theft or preventing the rightful owner from withdrawing.

• Test assertion: The attacker’s balance increases by the corresponding amount, while the victim’s (rejecting’s) failedTransferCredits remain unchanged.

Recommended Mitigation

• Safest / most straightforward: Only allow the mapped holder to withdraw (i.e., msg.sender must equal _receiver), and clear the mapping before sending funds (checks-effects-interactions).

• More flexible: Allow the owner to withdraw on behalf of others, but ensure the mapping is properly cleared and appropriate permission checks are in place.