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

6 stars 4 forks source link

0xpep7 - Incomplete Error Recovery Mechanism in DelayedWETH.hold function #209

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

0xpep7

medium

Incomplete Error Recovery Mechanism in DelayedWETH.hold function

Summary

DelayedWETH.hold is intended to allow the owner to recover from error cases by pulling ETH from a specific owner. However, a vulnerability exists in the function's logic, potentially allowing users to front-run upcoming transactions and redeem their underlying ETH from WETH if a certain time has passed since the error occurred.

Vulnerability Detail

The vulnerability lies in the hold function, where the logic to "pull ETH" from a specific owner is missing. Instead, the function merely sets an allowance without executing the necessary steps to transfer ETH to the owner in case of an error.

Findings are labeled with '<= FOUND'

// File: src/dispute/interfaces/IDelayedWETH.sol
42:    function recover(uint256 _wad) external;
43:
44:    /// @notice Allows the owner to recover from error cases by pulling ETH from a specific owner. // <= FOUND
45:    /// @param _guy The address to recover the WETH from.
46:    /// @param _wad The amount of WETH to recover.
47:    function hold(address _guy, uint256 _wad) external;
...

// File: src/dispute/weth/DelayedWETH.sol
92:    function hold(address _guy, uint256 _wad) external { // <= FOUND: the `hold` function just set allowance without pulling WETH out of `_guy` balance
93:        require(msg.sender == owner(), "DelayedWETH: not owner");
94:        allowance[_guy][msg.sender] = _wad;
95:        emit Approval(_guy, msg.sender, _wad);
96:    }

Impact

The impact of this vulnerability is significant as it compromises the contract's error recovery mechanism. Without the proper logic to pull ETH from the designated owner, users may exploit the system by front-running transactions and redeeming their underlying ETH from WETH prematurely, potentially leading to financial losses or disruption of the contract's intended functionality.

Code Snippet

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

Tool used

Manual Review

Recommendation

To address this vulnerability, it is recommended to modify the hold function to include the necessary logic for pulling ETH from the designated _guy's balance. Specifically, the function should execute an ETH transferFrom call to actually recover error funds.

Duplicate of #193