Withdrawal Delay Improperly Enforced

Summary

Improper logic allows withdrawals inside of specified delay period rather than outside

Vulnerability Detail

In the contest handbook, a clause in the Risks and Mitigation section of FaultDisputeGame states the following:

4. To mitigate bond loss, the contract should ensure that ETH can only leave through the intended claimCredit function. Additionally, the DelayedWETH contract is used to force withdrawal declarations with a delay, allowing time for off-chain review of game results and bond distribution.

For a bond to be withdrawn it needs to first be unlocked, and this is done through calls to _distributeBond() in FaultDisputeGame.sol:

function _distributeBond(address _recipient, ClaimData storage _bonded) internal {
        // Set all bits in the bond value to indicate that the bond has been paid out.
        uint256 bond = _bonded.bond;
        if (bond == CLAIMED_BOND_FLAG) revert ClaimAlreadyResolved();
        _bonded.bond = CLAIMED_BOND_FLAG;

        // Increase the recipient's credit.
        credit[_recipient] += bond;

        // Unlock the bond.
        WETH.unlock(_recipient, bond);
    }

This function is called in the same FaultDisputeGame.sol contract during calls to resolveClaim() which resolves a subGame. The WETH contract used is a modified version called DelayedWETH which enforces a delay period between the calls to unlock() and withdraw(). This delay period is needed to create a window for off-chain review of game results and bond distribution. But within the withdraw() function of DelayedWETH an improper require check is used:

require(wd.timestamp + DELAY_SECONDS <= block.timestamp, "DelayedWETH: withdrawal delay not met");

This goes against the clause in the Risks and Mitigations section of DelayedWETH in the contest handbook:

1. The main risk is that the DelayedWETH contract does not function as intended, allowing the FaultDisputeGame contract to withdraw ETH without waiting for the delay period.
2. The FaultDisputeGame contract must integrate with DelayedWETH correctly, ensuring that the withdrawals mapping is a hard accounting method and cannot be bypassed.

While this period is measured in seconds, given block times of 13 seconds - there exists a small window which allows a withdrawal to be processed within the delay period instead of after the delay period as specified by the docs.

Impact

A recipient can claim credit sooner than specified in the documentation.

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/f216b0d3ad08c1a0ead557ea74691aaefd5fd489/optimism/packages/contracts-bedrock/src/dispute/weth/DelayedWETH.sol#L79

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

require(wd.timestamp + DELAY_SECONDS <= block.timestamp, "DelayedWETH: withdrawal delay not met");

Tool used

Manual Review

Recommendation

Switch the comparison from <= to >=

require(wd.timestamp + DELAY_SECONDS >= block.timestamp, "DelayedWETH: withdrawal delay not met");

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

Z3R0 - Withdrawal Delay Improperly Enforced #153