Merge pull request #11 from ethereum-optimism/tip/pcw109550/fix-preim…

…age-oracle-off-by-one fix: PreimageOracle off-by-one
ethereum-optimism · Feb 13, 2024 · d1c91bc · d1c91bc
2 parents 4e0e36a + 0b86f77
commit d1c91bc
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 2 deletions.
diff --git a/rvsol/src/PreimageOracle.sol b/rvsol/src/PreimageOracle.sol
@@ -74,8 +74,8 @@ contract PreimageOracle is IPreimageOracle {
             // len(sig) + len(partOffset) + len(preimage offset) = 4 + 32 + 32 = 0x44
             size := calldataload(0x44)
 
-            // revert if part offset > size+8 (i.e. parts must be within bounds)
-            if gt(_partOffset, add(size, 8)) {
+            // revert if part offset >= size+8 (i.e. parts must be within bounds)
+            if iszero(lt(_partOffset, add(size, 8))) {
                 // Store "PartOffsetOOB()"
                 mstore(0, 0xfe254987)
                 // Revert with "PartOffsetOOB()"

diff --git a/rvsol/test/PreimageOracle.t.sol b/rvsol/test/PreimageOracle.t.sol
@@ -133,6 +133,16 @@ contract PreimageOracle_Test is Test {
         assertTrue(ok);
     }
 
+    /// @notice Tests that adding a global keccak256 pre-image at the part boundary reverts.
+    function test_loadKeccak256PreimagePart_partBoundary_reverts() public {
+        bytes memory preimage = hex"deadbeef";
+        uint256 offset = preimage.length + 8;
+
+        // TODO: remove magic errors
+        vm.expectRevert(0xfe254987);
+        oracle.loadKeccak256PreimagePart(offset, preimage);
+    }
+
     /// @notice Tests that a pre-image cannot be set with an out-of-bounds offset.
     function test_loadLocalData_outOfBoundsOffset_reverts() public {
         bytes32 preimage = bytes32(uint256(0xdeadbeef));