sherlock-audit / 2023-01-optimism-judging

24 stars 10 forks source link

ak1 - validate the `outputRoot` consistently #250

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

ak1

medium

validate the outputRoot consistently

Summary

We observed that the outputRoot is not validated in all the places.

while the outputRoot is validated in L2OutputOracle.sol in Line during the proposeL2Output But, not in OptimismPortal.sol in the function proveWithdrawalTransaction. refer the lines

Vulnerability Detail

Refer the summary section.

Impact

Since the function proveWithdrawalTransaction is external, malicious user could call the proveWithdrawalTransaction function by crafting the the input parameters in following places which will not revert in all the validating places.

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L193-L223

As a result, there will be plenty of transactions loaded in provenWithdrawals mapping.

provenWithdrawals is public function, the values will be stored in contract state which could consume gas by sitting idle. No one will be there to prove and complete the transaction.

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L176-L186

Tool used

Manual Review

Recommendation

We suggest to validate the outputRoot consistently in all the places.

rcstanciu commented 1 year ago

Comment from Optimism


Description: Validate the outputRoot consistently

Reason: This is a non-issue. The proveWithdrawalTransaction function validates that the _l2OutputIndex passed corresponds to an existing output proposal in the L2OutputOracle and that the _outputRootProof passed hashes to the outputRoot returned by the L2OutputOracle. In addition, the _tx passed is verified for inclusion within the _outputRootProof's messagePasserStorageRoot via the passed _withdrawalProof.