Merge pull request #35 from lightclient/fix-audit

lightclient · web-flow · commit 1d679164e03d · 2024-11-18T17:17:16.000+08:00
all: prague audit fixes
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -27,8 +27,8 @@ jobs:
       - name: Run Forge build
         run: |
           forge --version
-          ./build-wrapper build --sizes
+          forge build --sizes
         id: build
       - name: Run Forge tests
-        run: ./build-wrapper test -vvv
+        run: forge test -vvv
         id: test
diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,2 @@
 cache/
 out/
-test/*.t.sol
diff --git a/.gitmodules b/.gitmodules
@@ -1,3 +1,6 @@
 [submodule "lib/forge-std"]
 	path = lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
+[submodule "lib/geas-ffi"]
+	path = lib/geas-ffi
+	url = https://github.com/lightclient/geas-ffi
diff --git a/README.md b/README.md
@@ -26,10 +26,10 @@ $ geas src/withdrawals/main.eas
 
 ## Testing
 
-The tests can be executed using the `build-wrapper` script with the same arguments as [forge][forge]:
+The tests can be executed using [forge][forge]:
 
 ```console
-$ ./build-wrapper test
+$ forge test
 [⠒] Compiling...
 [⠒] Compiling 1 files with 0.8.14
 [⠢] Solc 0.8.14 finished in 976.49ms
diff --git a/build-wrapper b/build-wrapper
diff --git a/foundry.toml b/foundry.toml
@@ -1,4 +1,6 @@
 [profile.default]
+evm_version = 'prague'
 src = 'src'
 out = 'out'
+ffi = true
 libs = ['lib']
diff --git a/lib/geas-ffi b/lib/geas-ffi
@@ -0,0 +1 @@
+Subproject commit d18faf3b81dbd1cc89f617df8accac25ca10e9d9
diff --git a/src/common/fake_expo.eas b/src/common/fake_expo.eas
@@ -43,3 +43,9 @@ swap1       ;; [accum, output, i, numer, denom]
 swap4       ;; [denom, output, i, numer, accum]
 swap1       ;; [output, denom, i, numer, accum]
 div         ;; [output / denom, i , numer, accum]
+
+;; clean up stack
+swap3       ;; [accum, i, numer, result]
+pop         ;; [i, numer, result]
+pop         ;; [numer, result]
+pop         ;; [result]
diff --git a/src/consolidations/main.eas b/src/consolidations/main.eas
@@ -201,7 +201,7 @@ handle_input:
 ;; reset.
 read_requests:
   ;; Determine the size of the queue by calculating tail - head.
-  push QUEUE_TAIL       ;; [tail_idx_slot, head_idx, head_idx]
+  push QUEUE_TAIL       ;; [tail_idx_slot]
   sload                 ;; [tail_idx]
   push QUEUE_HEAD       ;; [head_idx_slot, tail_idx]
   sload                 ;; [head_idx, tail_idx]
@@ -369,7 +369,7 @@ update_excess:
   iszero                ;; [inhibitor != excess, excess, count]
   jumpi @skip_reset     ;; [excess, count]
 
-  ;; Drop the excess from storage and use 0.
+  ;; Drop the excess from stack and use 0.
   pop                   ;; [count]
   push 0                ;; [reset_excess, count]
 
diff --git a/src/execution_hash/main.eas b/src/execution_hash/main.eas
@@ -14,16 +14,9 @@
 ;; BUFLEN returns the HISTORY_BUFFER_LENGTH as defined in the EIP.
 #define BUFLEN 8191
 
-;; SYSADDR is the address which calls the contract to submit a new root.
+;; SYSADDR is the address which calls the contract to submit a new block hash.
 #define SYSADDR 0xfffffffffffffffffffffffffffffffffffffffe
 
-;; do_revert sets up and then executes a revert(0,0) operation.
-#define %do_revert() {
-  push 0            ;; [0]
-  push 0            ;; [0, 0]
-  revert            ;; []
-}
-
 ;; ----------------------------------------------------------------------------
 ;; MACROS END -----------------------------------------------------------------
 ;; ----------------------------------------------------------------------------
@@ -37,20 +30,18 @@
   jumpi @submit     ;; []
 
   ;; Fallthrough if addresses don't match -- this means the caller intends
-  ;; to read a root.
+  ;; to read a block hash.
 
   ;; Verify input is 32 bytes long.
   push 32           ;; [32]
   calldatasize      ;; [calldatasize, 32]
-  eq                ;; [calldatasize == 32]
+  sub               ;; [calldatasize - 32]
 
   ;; Jump to continue if length-check passed, otherwise revert.
-  jumpi @load       ;; []
-  %do_revert()      ;; []
+  jumpi @throw      ;; []
 
-load:
-  ;; Check if input is requesting a block hash greater than current block
-  ;; number.
+  ;; Check if input is requesting a block hash greater than current block number
+  ;; minus 1.
   push 0            ;; [0]
   calldataload      ;; [input]
   push 1            ;; [1, input]
@@ -62,13 +53,11 @@ load:
 
   ;; Check if the input is requesting a block hash before the earliest available
   ;; hash currently. Since we've verfied that input <= number - 1, it's safe to
-  ;; check the following:
-  ;; number - 1 - input <= BUFLEN, which also equals: number - input < BUFLEN
-  dup1              ;; [input, input]
-  number            ;; [number, input, input]
-  sub               ;; [number - input, input]
-  push BUFLEN+1     ;; [buflen, number - input, input]
-  lt                ;; [buflen < number - input, input]
+  push BUFLEN       ;; [buflen, input]
+  dup2              ;; [input, buflen, input]
+  number            ;; [number, input, buflen, input]
+  sub               ;; [number - input, buflen, input]
+  gt                ;; [number - input > buflen, input]
   jumpi @throw      ;; [input]
 
   ;; Load the hash.
@@ -86,8 +75,9 @@ load:
 
 throw:
   ;; Reverts current execution with no return data.
-  pop
-  %do_revert()
+  push 0            ;; [0]
+  push 0            ;; [0, 0]
+  revert            ;; []
 
 submit:
   push 0            ;; [0]
diff --git a/src/withdrawals/main.eas b/src/withdrawals/main.eas
@@ -196,7 +196,7 @@ handle_input:
 ;; request count is reset.
 read_requests:
   ;; Determine the size of the queue by calculating tail - head.
-  push QUEUE_TAIL       ;; [tail_idx_slot, head_idx, head_idx]
+  push QUEUE_TAIL       ;; [tail_idx_slot]
   sload                 ;; [tail_idx]
   push QUEUE_HEAD       ;; [head_idx_slot, tail_idx]
   sload                 ;; [head_idx, tail_idx]
@@ -366,7 +366,7 @@ update_excess:
   iszero                ;; [inhibitor != excess, excess, count, count]
   jumpi @skip_reset     ;; [excess, count, count]
 
-  ;; Drop the excess from storage and use 0.
+  ;; Drop the excess from stack and use 0.
   pop                   ;; [count, count]
   push 0                ;; [reset_excess, count]
 
diff --git a/test/BeaconRoot.t.sol b/test/BeaconRoot.t.sol
@@ -2,9 +2,10 @@
 pragma solidity ^0.8.13;
 
 import "forge-std/Test.sol";
+import "geas-ffi/Geas.sol";
 import "../src/Contract.sol";
 
-address constant addr = 0x000000000000000000000000000000000000000b;
+address constant addr = 0x0000000000000000000000000000000000000bBB;
 address constant sysaddr = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE;
 uint256 constant buflen = 8191;
 bytes32 constant root    = hex"88e96d4537bea4d9c05d12549907b32561d3bf31f45aae734cdc119f13406cb6";
@@ -25,7 +26,7 @@ contract ContractTest is Test {
     address unit;
 
     function setUp() public {
-        vm.etch(addr, hex"@bytecode@");
+        vm.etch(addr, Geas.compile("src/beacon_root/main.eas"));
         unit = addr;
     }
 
diff --git a/test/Consolidation.t.sol b/test/Consolidation.t.sol
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.13;
 
+import "geas-ffi/Geas.sol";
 import "./Test.sol";
 
 uint256 constant target_per_block = 1;
@@ -10,8 +11,8 @@ uint256 constant inhibitor = uint256(bytes32(0xfffffffffffffffffffffffffffffffff
 contract ConsolidationTest is Test {
 
   function setUp() public {
-    vm.etch(addr, hex"@bytecode@");
-    vm.etch(fakeExpo, hex"@bytecode_expo@");
+    vm.etch(addr, Geas.compile("src/consolidations/main.eas"));
+    vm.etch(fakeExpo, Geas.compile("src/common/fake_expo_test.eas"));
   }
 
   // testInvalidRequest checks that common invalid requests are rejected.
diff --git a/test/ExecutionHash.t.sol b/test/ExecutionHash.t.sol
@@ -2,6 +2,7 @@
 pragma solidity ^0.8.13;
 
 import "forge-std/Test.sol";
+import "geas-ffi/Geas.sol";
 import "../src/Contract.sol";
 
 address constant addr = 0x000000000000000000000000000000000000aaaa;
@@ -18,40 +19,37 @@ function hash_idx() view returns (bytes32) {
 }
 
 contract ContractTest is Test {
-    address unit;
-
     function setUp() public {
-        vm.etch(addr, hex"@bytecode@");
-        unit = addr;
+        vm.etch(addr, Geas.compile("src/execution_hash/main.eas"));
     }
 
     // testRead verifies the contract returns the expected execution hash.
     function testExecRead() public {
         // Store hash at expected indexes.
-        vm.store(unit, hash_idx(), hash);
+        vm.store(addr, hash_idx(), hash);
 
         // Read hash associated with current timestamp.
-        (bool ret, bytes memory data) = unit.call(bytes.concat(lastBlockNumber()));
+        (bool ret, bytes memory data) = addr.call(bytes.concat(lastBlockNumber()));
         assertTrue(ret);
         assertEq(data, bytes.concat(hash));
     }
 
     function testReadBadCalldataSize() public {
         // Store hash at expected indexes.
-        vm.store(unit, hash_idx(), hash);
+        vm.store(addr, hash_idx(), hash);
 
         // Call with 0 byte arguement.
-        (bool ret, bytes memory data) = unit.call(hex"");
+        (bool ret, bytes memory data) = addr.call(hex"");
         assertFalse(ret);
         assertEq(data, hex"");
 
         // Call with 31 byte arguement.
-        (ret, data) = unit.call(hex"00000000000000000000000000000000000000000000000000000000001337");
+        (ret, data) = addr.call(hex"00000000000000000000000000000000000000000000000000000000001337");
         assertFalse(ret);
         assertEq(data, hex"");
 
         // Call with 33 byte arguement.
-        (ret, data) = unit.call(hex"000000000000000000000000000000000000000000000000000000000000001337");
+        (ret, data) = addr.call(hex"000000000000000000000000000000000000000000000000000000000000001337");
         assertFalse(ret);
         assertEq(data, hex"");
     }
@@ -62,20 +60,25 @@ contract ContractTest is Test {
         uint256 number = block.number-1;
 
         // Store hash at expected indexes.
-        vm.store(unit, hash_idx(), hash);
+        vm.store(addr, hash_idx(), hash);
+
+        // Request current block.
+        (bool ret, bytes memory data) = addr.call(bytes.concat(bytes32(block.number)));
+        assertFalse(ret);
+        assertEq(data, hex"");
 
         // Wrap around buflen once forward.
-        (bool ret, bytes memory data) = unit.call(bytes.concat(bytes32(number+buflen)));
+        (ret, data) = addr.call(bytes.concat(bytes32(number+buflen)));
         assertFalse(ret);
         assertEq(data, hex"");
 
         // Wrap around buflen once backward.
-        (ret, data) = unit.call(bytes.concat(bytes32(number-buflen-1)));
+        (ret, data) = addr.call(bytes.concat(bytes32(number-buflen)));
         assertFalse(ret);
         assertEq(data, hex"");
 
         // Block number zero should fail.
-        (ret, data) = unit.call(bytes.concat(bytes32(0)));
+        (ret, data) = addr.call(bytes.concat(bytes32(0)));
         assertFalse(ret);
         assertEq(data, hex"");
     }
@@ -84,12 +87,12 @@ contract ContractTest is Test {
     function testUpdate() public {
         // Simulate pre-block call to set hash.
         vm.prank(sysaddr);
-        (bool ret, bytes memory data) = unit.call(bytes.concat(hash));
+        (bool ret, bytes memory data) = addr.call(bytes.concat(hash));
         assertTrue(ret);
         assertEq(data, hex"");
 
         // Verify hash.
-        bytes32 got = vm.load(unit, hash_idx());
+        bytes32 got = vm.load(addr, hash_idx());
         assertEq(got, hash);
     }
 
@@ -105,13 +108,13 @@ contract ContractTest is Test {
 
             // Simulate pre-block call to set hash.
             vm.prank(sysaddr);
-            (bool ret, bytes memory data) = unit.call(bytes.concat(pbbr));
+            (bool ret, bytes memory data) = addr.call(bytes.concat(pbbr));
             assertTrue(ret);
             assertEq(data, hex"");
 
             // Call contract as normal account to get exeuction hash associated
             // with current timestamp.
-            (ret, data) = unit.call(bytes.concat(lastBlockNumber()));
+            (ret, data) = addr.call(bytes.concat(lastBlockNumber()));
             assertTrue(ret);
             assertEq(data, bytes.concat(pbbr));
 
@@ -131,16 +134,23 @@ contract ContractTest is Test {
         for (uint256 i = 0; i < buflen; i += 1) {
             bytes32 pbbr = bytes32(i*1337);
             vm.prank(sysaddr);
-            (bool ret, bytes memory data) = unit.call(bytes.concat(pbbr));
+            (bool ret, bytes memory data) = addr.call(bytes.concat(pbbr));
             assertTrue(ret);
             assertEq(data, hex"");
-            vm.roll(block.number+1);
+            if (i+1 < buflen) {
+              // Only bump block number if not last iteration.
+              vm.roll(block.number+1);
+            }
         }
 
+        // Begin reading before start block, since roots are written for
+        // previous.
+        uint256 base = start-1;
+
         // Attempt to read all values in same block context.
         for (uint256 i = 0; i < buflen; i += 1) {
-            bytes32 num = bytes32(uint256(start+i-1));
-            (bool ret, bytes memory got) = unit.call(bytes.concat(num));
+            bytes32 num = bytes32(uint256(base+i));
+            (bool ret, bytes memory got) = addr.call(bytes.concat(num));
             assertTrue(ret);
             assertEq(got, bytes.concat(bytes32(i*1337)));
         }
diff --git a/test/FakeExpo.t.sol b/test/FakeExpo.t.sol
@@ -1,11 +1,12 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.13;
 
+import "geas-ffi/Geas.sol";
 import "./Test.sol";
 
 contract FakeExpoTest is Test {
   function setUp() public {
-    vm.etch(fakeExpo, hex"@bytecode@");
+    vm.etch(fakeExpo, Geas.compile("src/common/fake_expo_test.eas"));
   }
 
   // testFakeExpo calls the fake exponentiation logic with specific values.
diff --git a/test/Withdrawal.t.sol b/test/Withdrawal.t.sol

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,2 @@`
`1`	`1`	`cache/`
`2`	`2`	`out/`
`3`		`-test/*.t.sol`