sherlock-audit / 2024-02-optimism-2024-judging

6 stars 4 forks source link

0xpep7 - Continuous DelayedWETH.unlock Calls With Short Interval Would Extends Previous Unclaimed Withdrawals #214

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

0xpep7

medium

Continuous DelayedWETH.unlock Calls With Short Interval Would Extends Previous Unclaimed Withdrawals

Summary

The audit report focuses on the unbonding mechanism of the smart contract. A vulnerability has been identified wherein continuous unlocking can extend previously unclaimed withdrawals, potentially compromising the core functionality of the unbonding mechanism.

Vulnerability Detail

The vulnerability arises from the continuous unlock process, which extends previously unclaimed withdrawals without proper checks. As a result, the unbonding mechanism fails to function as intended, potentially locking the withdrawal process for specific sub-accounts indefinitely.

// File: src/dispute/weth/DelayedWETH.sol
57:    function unlock(address _guy, uint256 _wad) external {
        ...
62:        // leaky abstraction, so we chose this instead.
63:        WithdrawalRequest storage wd = withdrawals[msg.sender][_guy];
64:        wd.timestamp = block.timestamp;// <= FOUND: continuous unlock will extend previously unclaimed withdrawals
65:        wd.amount += _wad;
66:    }

Impact

The impact of this vulnerability is severe as it undermines the core functionality of the unbonding mechanism. By continuously unlocking funds, users' eligible withdrawals could be stuck for an amount of time, potentially indefinitely if the unlock interval is shorter than the delayed period.

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/tree/main/optimism/packages/contracts-bedrock//src/dispute/weth/DelayedWETH.sol#L64

Tool used

Manual Review

Recommendation

To address this vulnerability, it is recommended to introduce another property to the WithdrawalRequest struct, such as "unclaimed." When a new unlock is created, the contract should check if DELAY_SECONDS has passed and move the "amount" to "unclaimed" accordingly. This ensures that continuous unlocks do not extend previously unclaimed withdrawals and maintains the integrity of the unbonding mechanism.

Duplicate of #195