sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

0 stars 0 forks source link

safdie - Race conditions/cooldown manipulation on `withdraw` function may allow an attacker to manipulate or bypass the cooldown period in `AccountFacetImpl.sol` #6

Open sherlock-admin4 opened 1 week ago

sherlock-admin4 commented 1 week ago

safdie

High

Race conditions/cooldown manipulation on withdraw function may allow an attacker to manipulate or bypass the cooldown period in AccountFacetImpl.sol

Summary

The vulnerability in the withdraw() function, which relies on withdrawCooldown, may allow an attacker to manipulate or bypass the cooldown period through race conditions. If two or more transactions interact with the contract at almost the same time, it could allow the attacker to bypass the intended delay imposed by the cooldown mechanism and withdraw more than intended or more frequently than allowed.

Root Cause

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/Account/AccountFacetImpl.sol#L29-L32 The vulnerability occurs because the withdrawCooldown is updated at the end of some operations, and in certain scenarios, it may not prevent rapid withdrawals if there are multiple parallel executions or transaction ordering (also known as front-running).

Internal pre-conditions

Let's assume that an attacker can execute two transactions (Tx1 and Tx2) in rapid succession before the state is updated with the new cooldown time.

Steps:

  1. Tx1: The attacker initiates a valid withdrawal call. The cooldown period hasn't been reached, and the withdrawal is processed. However, the state hasn't yet been updated due to a delay in the transaction being mined and confirmed. Before withdrawCooldown[msg.sender] is updated, the attacker sends another transaction.
  2. Tx2: The attacker sends another withdrawal transaction during the time gap before the state is updated (front-running or racing the withdrawCooldown update). Because both transactions may be processed in the same block or in close proximity, Tx2 checks the same withdrawCooldown, finds it valid, and allows another withdrawal to proceed.
  3. As a result, the attacker withdraws twice (or more) before the cooldown is enforced.

External pre-conditions

No response

Attack Path

By running PoC below, the attacker would attempt to perform multiple withdrawals in rapid succession before the withdrawCooldown state is updated.

Impact

The cooldown mechanism is crucial to prevent rapid withdrawals. By bypassing this period through a race condition, the attacker can withdraw funds much faster than intended, which could lead to significant financial damage, especially in systems with large amounts of user funds. If an attacker is able to front-run or bypass the cooldown, they may drain their balance or even other users' balances if the system is poorly secured. This can destabilize the entire platform if users are allowed to deplete their funds without following cooldown rules.

PoC

// Attacker code:
// Assuming we have web3.js or ethers.js to send raw transactions quickly.

async function raceConditionExploit(contractInstance, attackerAddress) {
  // Setup parameters
  const amount = web3.utils.toWei("100", "ether");  // Example amount

  // Send two withdrawal transactions almost simultaneously
  const tx1 = contractInstance.methods.withdraw(attackerAddress, amount).send({ from: attackerAddress });
  const tx2 = contractInstance.methods.withdraw(attackerAddress, amount).send({ from: attackerAddress });

  // Race both transactions
  await Promise.all([tx1, tx2]);

  console.log("Both transactions executed!");
}

Mitigation

  1. Ensure that no state-changing functions can be executed in rapid succession. OpenZeppelin’s ReentrancyGuard can prevent multiple calls in the same transaction.
  2. Instead of checking block.timestamp directly, implement a mechanism that ensures the cooldown is strictly enforced, even under front-running or block timestamp manipulation attempts.
  3. Move the update of withdrawCooldown to the very beginning of the withdraw() function, ensuring that subsequent withdrawal attempts cannot be processed before the state change is confirmed.
  4. Implement daily or periodic withdrawal limits, adding an extra layer of protection against such attacks.