fix: Check valid range for s and add test (#363)

0xPolygonZero · Jul 5, 2024 · 043ab68 · 043ab68
1 parent 653b7e4
commit 043ab68
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 1 deletion.
diff --git a/evm_arithmetization/src/cpu/kernel/asm/curve/secp256k1/ecrecover.asm b/evm_arithmetization/src/cpu/kernel/asm/curve/secp256k1/ecrecover.asm
@@ -175,6 +175,10 @@ pubkey_to_addr:
     PUSH @SECP_SCALAR
 %endmacro
 
+%macro secp_scalar_half
+    PUSH @SECP_SCALAR_HALF
+%endmacro
+
 // Return u256::MAX which is used to indicate the input was invalid.
 %macro ecrecover_invalid_input
     // stack: retdest

diff --git a/evm_arithmetization/src/cpu/kernel/asm/transactions/common_decoding.asm b/evm_arithmetization/src/cpu/kernel/asm/transactions/common_decoding.asm
@@ -153,6 +153,14 @@
     // stack: rlp_addr
     %decode_rlp_scalar
     %stack (rlp_addr, s) -> (s, rlp_addr)
+
+    // EIP-2: Check that s is within valid range.
+    DUP1
+    %secp_scalar_half
+    // stack: ceil(N/2), s, s, rlp_addr
+    %assert_gt
+
+    // stack: s, rlp_addr
     %mstore_txn_field(@TXN_FIELD_S)
     // stack: rlp_addr
 %endmacro

diff --git a/evm_arithmetization/src/cpu/kernel/constants/mod.rs b/evm_arithmetization/src/cpu/kernel/constants/mod.rs
@@ -129,7 +129,7 @@ const HASH_CONSTANTS: [(&str, [u8; 32]); 2] = [
     ),
 ];
 
-const EC_CONSTANTS: [(&str, [u8; 32]); 24] = [
+const EC_CONSTANTS: [(&str, [u8; 32]); 25] = [
     (
         "U256_MAX",
         hex!("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
@@ -204,6 +204,11 @@ const EC_CONSTANTS: [(&str, [u8; 32]); 24] = [
         "SECP_SCALAR",
         hex!("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141"),
     ),
+    (
+        "SECP_SCALAR_HALF",
+        // Corresponds to `ceil(SECP_SCALAR / 2)`.
+        hex!("7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a1"),
+    ),
     (
         "SECP_GLV_BETA",
         hex!("7ae96a2b657c07106e64479eac3434e99cf0497512f58995c1396c28719501ee"),

diff --git a/evm_arithmetization/src/cpu/kernel/tests/transaction_parsing/parse_type_0_txn.rs b/evm_arithmetization/src/cpu/kernel/tests/transaction_parsing/parse_type_0_txn.rs
@@ -68,3 +68,22 @@ fn process_type_0_txn() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn process_type_0_txn_invalid_sig() -> Result<()> {
+    let process_type_0_txn = KERNEL.global_labels["process_type_0_txn"];
+    let process_normalized_txn = KERNEL.global_labels["process_normalized_txn"];
+
+    let retaddr = 0xDEADBEEFu32.into();
+    let mut interpreter: Interpreter<F> = Interpreter::new(process_type_0_txn, vec![retaddr]);
+
+    // Same transaction as `process_type_0_txn()`, with the exception that the `s`
+    // component in the signature is flipped (i.e. `s' = N - s`, where `N` is the
+    // order of the SECP256k1 prime subgroup).
+    interpreter.extend_memory_segment_bytes(Segment::RlpRaw, hex!("f861050a8255f0940000000000000000000000000000000000000000648242421ca07c5c61ed975ebd286f6b027b8c504842e50a47d318e1e801719dd744fe93e6c6a0e184aee64a822ab1e8a00d0faa36e0c408f99e2ca41c87ec8b557e9be8f0949f").to_vec());
+
+    let result = interpreter.run();
+    assert!(result.is_err());
+
+    Ok(())
+}