There’s an existing logic to prevent the CHALLENGER from deleting a l2Output after the finalization period has ended.
This is done to prevent having user withdrawals blocked after the finalization period has elapsed without challenges. The recommendation to prevent the CHALLENGER from deleting l2Outputs after the finalization period was made on a previous audit contest.
However, the Optimism Bedrock documentation mentions that the delay period for a user withdrawal is 7 days.
“Users wait seven days for the withdrawal to be finalized.”
Therefore, users will be expecting a seven days withdrawal delay period. However, there’s no guarantee in the code that the finalization period to prevent the CHALLENGER from delete an l2Output will actually be 7 days.
In fact, the FINALIZATION_PERIOD_SECONDS could be set to any value. Even if FINALIZATION_PERIOD_SECONDS is an immutable variable we have the following comment highlighting that these values can be updated.
“It can safely be modified by upgrading the implementation contract”.
Hence, it’s possible to update the FINALIZATION_PERIOD_SECONDS in a way that prevents user withdrawals after the 7 days period.
The following scenario can occur:
Another scenario, which wouldn’t involve any proxy or update, would be for FINALIZATION_PERIOD_SECONDS to be set with a value bigger than 7 days in the first place.
If a user cannot make a withdrawal after 7 days, the protocol guarantees are lost.
Although there might be other ways for the project to manipulate the system with proxy updates. Leaving a non-constant FINALIZATION_PERIOD_SECONDS unnecessarily opens the door for the CHALLENGER to rug transactions in L2OutputOracle.deleteL2Outputs().
The 7 days period should increase the trust in the system, due to the increased security against bridge attacks. However, if the value is no longer 7 days and can be any amount of days, there’s no guarantee a user will be able to make his withdrawal since the CHALLENGER has the power to delete l2Outputs based on FINALIZATION_PERIOD_SECONDS.
I understand the likelihood of this happening is small. However, the impact is severe.
There’s no reason for Optimism/Base to manipulate FINALIZATION_PERIOD_SECONDS to be able to delete l2Outputs and brick honest users withdrawals. But currently users have to trust that Optimism/Base won’t do that.
The codebase should be built in a way that users don’t need to trust the system. Crypto protocols need to be trustless to be able to be different from traditional finance.
Furthermore, admin manipulation (bricking users withdrawals) is not mentioned to be out of scope in the contest readme.
I want to strongly reiterate that the focus on this report is not that finalization interval period might not be 7 days, but rather that since the value is not a constant it can be modified in a way that allows the CHALLENGER to delete l2Outputs and block users withdrawals even after a x amount of promised days (7 in the Bedrock documentation). The main focus of this report is that withdrawals can be bricked by the CHALLENGER after a period that was promised to the users to be safe.
Manual review, read Optimism documentation and study reports from previous Bedrock contests.
FINALIZATION_PERIOD_SECONDS should not be an immutable variable. As stated on the Proof of Concept section, the comment on OptimismPortal indicates that immutable variables will be updated at some point. Also as described previously, FINALIZATION_PERIOD_SECONDS may not even be set with 7 days initially since it can accept any value on the constructor.
The best solution would be to declare FINALIZATION_PERIOD_SECONDS as a constant with the fixed value of 7 days. This will provide users more assurances that the CHALLENGER won’t be able to delete l2Outputs after 7 days as it’s described on the Optimism documentation.
Other
The text was updated successfully, but these errors were encountered:
All reactions