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

6 stars 4 forks source link

guhu95 - Uninitialized `respectedGameType` may cause game type mismatch and fraudulent proofs #167

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 7 months ago

guhu95

medium

Uninitialized respectedGameType may cause game type mismatch and fraudulent proofs

Summary

The OptimismPortal2 contract sets the initial value of the respectedGameType state variable in the constructor instead of in the initializer as needed for a contract used with a proxy. If the contract is deployed with a non-zero GameType, such as PERMISSIONED_CANNON, it will cause a mismatch between the implementation and the proxy, causing the wrong game type to be respected by the proxy.

Vulnerability Detail

The OptimismPortal2 contract initializes the respectedGameType state variable in the constructor. However, since the contract is deployed as a proxy, the constructor is only executed on the implementation contract, not on the proxy itself.

If the OptimismPortal2 contract is deployed with a non-zero GameType, such as PERMISSIONED_CANNON (which has a GameType value of 1), the respectedGameType will be set to 1 on the implementation contract. However, the proxy contract, which is the one actually used, will have its respectedGameType set to the default value of 0, which corresponds to regular CANNON. This will be true for any initial GameType configuration that uses a non-zero value.

Notably, all three implemented game types are added to the factory during the chain upgrade, and the value of respectedGameType is neither checked nor set during the post-deployment configuration.

Impact

Consequently, in the above example, CANNON games will be respected by the proxy, contrary to the deployment intention. The difference between these games is evident in the name, as CANNON games can be created by anyone with any proposal. Additionally, these games are not likely to be monitored and disputed by the expected parties, since such a chain is not intended for permissionless proposals. This means that arbitrary and malicious proposals can be proven against the CANNON game type without any opposition. Attackers can exploit this vulnerability to prove fraudulent withdrawals, leading to the theft of funds from the Optimism bridge if the game type is not changed in time for withdrawal finalization.

That said, as there is a 7 day delay, and since honest withdrawals will not be provable initially due to the game type mismatch, it is unlikely that the mismatch will go undetected for the necessary 6 days (7 days minus the Guardian SLA of 24 hours). Thus, the likelihood of a catastrophic exploit is reduced by these safety measures, and the Guardian is likely to pause the contract in time.

However, there is also a DoS impact. This scenario can cause significant delays in the withdrawal process. The impact includes:

  1. Inability to prove honest withdrawals needs to be detected which is not immediate.
  2. Guardian will pause the contract, with an SLA of 24 hours, and call setRespectedGameType on the proxy contract.
  3. After setting the correct respectedGameType, dispute games need to be re-created because previously created games cannot be used for proving withdrawals due to the createdAt timestamp being earlier than respectedGameTypeUpdatedAt.
  4. The re-proving process and waiting for the required delays can take significant time due to the sequential nature of the above actions, denying users access to their funds for the duration of the additional delay.

Lastly, there is an impact of locking up bonds by the proposers in unusable games during this period (since the games need to be re-created).

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/main/optimism/packages/contracts-bedrock/src/L1/OptimismPortal2.sol#L134

Tool used

Manual Review

Recommendation

Move the initialization of the respectedGameType state variable from the constructor to the initializer.

Duplicate of #88

sherlock-admin2 commented 5 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/ethereum-optimism/optimism/pull/10198