fix: check the input lengths for secp256k1- functions

stacks-network · Mar 26, 2024 · dae33ab · dae33ab
1 parent 5edf193
commit dae33ab
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 2 deletions.
diff --git a/clar2wasm/src/linker.rs b/clar2wasm/src/linker.rs
@@ -3756,20 +3756,46 @@ fn link_secp256k1_recover_fn(linker: &mut Linker<ClarityWasmContext>) -> Result<
                     .and_then(|export| export.into_memory())
                     .ok_or(Error::Wasm(WasmError::MemoryNotFound))?;
 
+                let ret_ty =
+                    TypeSignature::new_response(BUFF_33.clone(), TypeSignature::UIntType).unwrap();
+                let repr_size = get_type_size(&ret_ty);
+
                 // Read the message bytes from the memory
                 let msg_bytes = read_bytes_from_wasm(memory, &mut caller, msg_offset, msg_length)?;
+                // To match the interpreter behavior, if the message is the
+                // wrong length, throw a runtime type error.
+                if msg_bytes.len() != 32 {
+                    return Err(CheckErrors::TypeValueError(
+                        BUFF_32.clone(),
+                        Value::buff_from(msg_bytes)?,
+                    )
+                    .into());
+                }
 
                 // Read the signature bytes from the memory
                 let sig_bytes = read_bytes_from_wasm(memory, &mut caller, sig_offset, sig_length)?;
+                // To match the interpreter behavior, if the signature is the
+                // wrong length, return a Clarity error.
+                if sig_bytes.len() != 65 {
+                    let result = Value::err_uint(2);
+                    write_to_wasm(
+                        caller,
+                        memory,
+                        &ret_ty,
+                        return_offset,
+                        return_offset + repr_size,
+                        &result,
+                        true,
+                    )?;
+                    return Ok(());
+                }
 
                 let result = match secp256k1_recover(&msg_bytes, &sig_bytes) {
                     Ok(pubkey) => Value::okay(Value::buff_from(pubkey.to_vec())?)?,
                     _ => Value::err_uint(1),
                 };
 
                 // Write the result to the return buffer
-                let ret_ty = TypeSignature::new_response(BUFF_33.clone(), TypeSignature::UIntType)?;
-                let repr_size = get_type_size(&ret_ty);
                 write_to_wasm(
                     caller,
                     memory,
@@ -3816,12 +3842,35 @@ fn link_secp256k1_verify_fn(linker: &mut Linker<ClarityWasmContext>) -> Result<(
 
                 // Read the message bytes from the memory
                 let msg_bytes = read_bytes_from_wasm(memory, &mut caller, msg_offset, msg_length)?;
+                // To match the interpreter behavior, if the message is the
+                // wrong length, throw a runtime type error.
+                if msg_bytes.len() != 32 {
+                    return Err(CheckErrors::TypeValueError(
+                        BUFF_32.clone(),
+                        Value::buff_from(msg_bytes)?,
+                    )
+                    .into());
+                }
 
                 // Read the signature bytes from the memory
                 let sig_bytes = read_bytes_from_wasm(memory, &mut caller, sig_offset, sig_length)?;
+                // To match the interpreter behavior, if the signature is the
+                // wrong length, return a Clarity error.
+                if sig_bytes.len() < 64 || sig_bytes.len() > 65 {
+                    return Ok(0i32);
+                }
 
                 // Read the public-key bytes from the memory
                 let pk_bytes = read_bytes_from_wasm(memory, &mut caller, pk_offset, pk_length)?;
+                // To match the interpreter behavior, if the public key is the
+                // wrong length, throw a runtime type error.
+                if pk_bytes.len() != 33 {
+                    return Err(CheckErrors::TypeValueError(
+                        BUFF_33.clone(),
+                        Value::buff_from(pk_bytes)?,
+                    )
+                    .into());
+                }
 
                 Ok(secp256k1_verify(&msg_bytes, &sig_bytes, &pk_bytes).map_or(0i32, |_| 1i32))
             },

diff --git a/clar2wasm/src/words/secp256k1.rs b/clar2wasm/src/words/secp256k1.rs
@@ -116,4 +116,46 @@ mod tests {
             0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
             0x03adb8de4bfb65db2cfd6120d55c6526ae9c52e675db7e47308636534ba7786110)", Ok(Some(Value::Bool(false))))
     }
+
+    #[test]
+    fn test_secp256k1_recover_bad_values() {
+        // For some reason, if the message-hash is the wrong size, it throws a
+        // runtime type error, but if the signature is the wrong size, it's a
+        // normal clarity error.
+
+        // Message hash too short
+        crosscheck("(secp256k1-recover? 0xde5b9eb9e7c5592930eb2e30a01369c36586d872082ed8181ee83d2a0ec20f
+            0x8738487ebe69b93d8e51583be8eee50bb4213fc49c767d329632730cc193b873554428fc936ca3569afc15f1c9365f6591d6251a89fee9c9ac661116824d3a1301)",
+            Err(()));
+
+        // Signature too short
+        crosscheck("(secp256k1-recover? 0xde5b9eb9e7c5592930eb2e30a01369c36586d872082ed8181ee83d2a0ec20f04
+            0x8738487ebe69b93d8e51583be8eee50bb4213fc49c767d1cc193b873554428fc936ca3569afc15f1c9365f6591d6251a89fee9c9ac661116824d3a13)",
+            Ok(Some(Value::err_uint(2))));
+    }
+
+    #[test]
+    fn test_secp256k1_verify_bad_values() {
+        // For some reason, if the message hash or public key are the wrong
+        // size, it throws a runtime type error, but if the signature is the
+        // wrong size, it's a normal clarity error.
+
+        // Message hash too short
+        crosscheck("(secp256k1-verify 0xde5b9eb9e7c5592930eb2e30a01369c36586d872082ed8181ee83d2a0ec20f
+            0x8738487ebe69b93d8e51583be8eee50bb4213fc49c767d329632730cc193b873554428fc936ca3569afc15f1c9365f6591d6251a89fee9c9ac661116824d3a1301
+            0x03adb8de4bfb65db2cfd6120d55c6526ae9c52e675db7e47308636534ba7786110)",
+            Err(()));
+
+        // Signature too short
+        crosscheck("(secp256k1-verify 0xde5b9eb9e7c5592930eb2e30a01369c36586d872082ed8181ee83d2a0ec20f04
+            0x8738487ebe69b93d8e51583be8eee50bb4213fc49c767d329632730cc193b873554428fc936ca3569afc15f1c9365f6591d6251a89fee9c9ac661116824d3a
+            0x03adb8de4bfb65db2cfd6120d55c6526ae9c52e675db7e47308636534ba7786110)",
+            Ok(Some(Value::Bool(false))));
+
+        // Public key is too short
+        crosscheck("(secp256k1-verify 0xde5b9eb9e7c5592930eb2e30a01369c36586d872082ed8181ee83d2a0ec20f04
+            0x8738487ebe69b93d8e51583be8eee50bb4213fc49c767d329632730cc193b873554428fc936ca3569afc15f1c9365f6591d6251a89fee9c9ac661116824d3a1301
+            0x03adb8de4bfb65db2cfd6120d55c6526ae9c52e675db7e47308636534ba77861)",
+            Err(()));
+    }
 }