fix: check canonical Fp elements

crates/precompile/src/bls12_381.rs

@@ -48,14 +48,15 @@ mod test {
     use serde_derive::{Deserialize, Serialize};
     use std::{fs, path::Path};
 
+    /// Test vector structure for BLS12-381 precompile tests.
     #[derive(Serialize, Deserialize, Debug)]
     #[serde(rename_all = "PascalCase")]
     struct TestVector {
         input: String,
-        expected: String,
+        expected: Option<String>,
         name: String,
-        gas: u64,
-        error: Option<bool>,
+        gas: Option<u64>,
+        expected_error: Option<String>,
     }
 
     #[derive(Serialize, Deserialize, Debug)]
@@ -67,6 +68,15 @@ mod test {
     }
 
     #[rstest]
+    #[case::fail_g1_add(g1_add::g1_add, "fail-add_G1_bls.json")]
+    #[case::fail_g1_mul(g1_mul::g1_mul, "fail-mul_G1_bls.json")]
+    #[case::fail_g1_msm(g1_msm::g1_msm, "fail-multiexp_G1_bls.json")]
+    #[case::fail_g2_add(g2_add::g2_add, "fail-add_G2_bls.json")]
+    #[case::fail_g2_mul(g2_mul::g2_mul, "fail-mul_G2_bls.json")]
+    #[case::fail_g2_msm(g2_msm::g2_msm, "fail-multiexp_G2_bls.json")]
+    #[case::fail_pairing(pairing::pairing, "fail-pairing_check_bls.json")]
+    #[case::fail_map_fp_to_g1(map_fp_to_g1::map_fp_to_g1, "fail-map_fp_to_G1_bls.json")]
+    #[case::fail_map_fp2_to_g2(map_fp2_to_g2::map_fp2_to_g2, "fail-map_fp2_to_G2_bls.json")]
     #[case::g1_add(g1_add::g1_add, "add_G1_bls.json")]
     #[case::g1_mul(g1_mul::g1_mul, "mul_G1_bls.json")]
     #[case::g1_msm(g1_msm::g1_msm, "multiexp_G1_bls.json")]
@@ -93,17 +103,23 @@ mod test {
             });
             let target_gas: u64 = 30_000_000;
             let res = precompile(&input, target_gas);
-            if vector.error.unwrap_or_default() {
-                assert!(res.is_err(), "expected error didn't happen in {test_name}");
+            if let Some(expected_error) = vector.expected_error {
+                assert!(res.is_err(), "expected error {expected_error} didn't happen in {test_name}, got result {res:?}");
             } else {
+                let Some(gas) = vector.gas else {
+                    panic!("gas is missing in {test_name}");
+                };
                 let (actual_gas, actual_output) =
                     res.unwrap_or_else(|e| panic!("precompile call failed for {test_name}: {e}"));
                 assert_eq!(
-                    vector.gas, actual_gas,
+                    gas, actual_gas,
                     "expected gas: {}, actual gas: {} in {test_name}",
-                    vector.gas, actual_gas
+                    gas, actual_gas
                 );
-                let expected_output = Bytes::from_hex(vector.expected).unwrap();
+                let Some(expected) = vector.expected else {
+                    panic!("expected output is missing in {test_name}");
+                };
+                let expected_output = Bytes::from_hex(expected).unwrap();
                 assert_eq!(
                     expected_output, actual_output,
                     "expected output: {expected_output}, actual output: {actual_output} in {test_name}");

crates/precompile/src/bls12_381/g1.rs

@@ -1,13 +1,14 @@
-use super::utils::{fp_to_bytes, remove_padding, PADDED_FP_LENGTH};
-use blst::{blst_fp_from_bendian, blst_p1_affine, blst_p1_affine_in_g1, blst_p1_affine_on_curve};
-use revm_primitives::{Bytes, PrecompileError};
+use super::utils::{fp_from_bendian, fp_to_bytes, remove_padding, PADDED_FP_LENGTH};
+use crate::primitives::{Bytes, PrecompileError};
+use blst::{blst_p1_affine, blst_p1_affine_in_g1, blst_p1_affine_on_curve};
 
 /// Length of each of the elements in a g1 operation input.
 pub(super) const G1_INPUT_ITEM_LENGTH: usize = 128;
+
 /// Output length of a g1 operation.
 const G1_OUTPUT_LENGTH: usize = 128;
 
-/// Encodes a G1 point in affine format into a byte slice with padded elements.
+/// Encodes a G1 point in affine format into byte slice with padded elements.
 pub(super) fn encode_g1_point(input: *const blst_p1_affine) -> Bytes {
     let mut out = vec![0u8; G1_OUTPUT_LENGTH];
     // SAFETY: out comes from fixed length array, input is a blst value.
@@ -18,6 +19,24 @@ pub(super) fn encode_g1_point(input: *const blst_p1_affine) -> Bytes {
     out.into()
 }
 
+/// Returns a `blst_p1_affine` from the provided byte slices, which represent the x and y
+/// affine coordinates of the point.
+///
+/// If the x or y coordinate do not represent a canonical field element, an error is returned.
+///
+/// See [fp_from_bendian] for more information.
+pub(super) fn decode_and_check_g1(
+    p0_x: &[u8; 48],
+    p0_y: &[u8; 48],
+) -> Result<blst_p1_affine, PrecompileError> {
+    let out = blst_p1_affine {
+        x: fp_from_bendian(p0_x)?,
+        y: fp_from_bendian(p0_y)?,
+    };
+
+    Ok(out)
+}
+
 /// Extracts a G1 point in Affine format from a 128 byte slice representation.
 ///
 /// NOTE: This function will perform a G1 subgroup check if `subgroup_check` is set to `true`.
@@ -34,13 +53,7 @@ pub(super) fn extract_g1_input(
 
     let input_p0_x = remove_padding(&input[..PADDED_FP_LENGTH])?;
     let input_p0_y = remove_padding(&input[PADDED_FP_LENGTH..G1_INPUT_ITEM_LENGTH])?;
-
-    let mut out = blst_p1_affine::default();
-    // SAFETY: input_p0_x and input_p0_y have fixed length, out is a blst value.
-    unsafe {
-        blst_fp_from_bendian(&mut out.x, input_p0_x.as_ptr());
-        blst_fp_from_bendian(&mut out.y, input_p0_y.as_ptr());
-    }
+    let out = decode_and_check_g1(input_p0_x, input_p0_y)?;
 
     if subgroup_check {
         // NB: Subgroup checks

crates/precompile/src/bls12_381/g1_msm.rs

@@ -11,6 +11,7 @@ use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G1MSM precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(g1_msm));
+
 /// BLS12_G1MSM precompile address.
 pub const ADDRESS: u64 = 0x0d;
 

crates/precompile/src/bls12_381/g1_mul.rs

@@ -37,8 +37,11 @@ pub(super) fn g1_mul(input: &Bytes, gas_limit: u64) -> PrecompileResult {
     // NB: Scalar multiplications, MSMs and pairings MUST perform a subgroup check.
     //
     // So we set the subgroup_check flag to `true`
-    let p0_aff = &extract_g1_input(&input[..G1_INPUT_ITEM_LENGTH], true)?;
+    let slice = &input[..G1_INPUT_ITEM_LENGTH];
+    let p0_aff = &extract_g1_input(slice, true)?;
+
     let mut p0 = blst_p1::default();
+
     // SAFETY: p0 and p0_aff are blst values.
     unsafe { blst_p1_from_affine(&mut p0, p0_aff) };
 

crates/precompile/src/bls12_381/g2.rs

@@ -1,13 +1,14 @@
-use super::utils::{fp_to_bytes, remove_padding, FP_LENGTH, PADDED_FP_LENGTH};
-use blst::{blst_fp_from_bendian, blst_p2_affine, blst_p2_affine_in_g2, blst_p2_affine_on_curve};
-use revm_primitives::{Bytes, PrecompileError};
+use super::utils::{fp_from_bendian, fp_to_bytes, remove_padding, FP_LENGTH, PADDED_FP_LENGTH};
+use crate::primitives::{Bytes, PrecompileError};
+use blst::{blst_fp2, blst_p2_affine, blst_p2_affine_in_g2, blst_p2_affine_on_curve};
 
 /// Length of each of the elements in a g2 operation input.
 pub(super) const G2_INPUT_ITEM_LENGTH: usize = 256;
+
 /// Output length of a g2 operation.
 const G2_OUTPUT_LENGTH: usize = 256;
 
-/// Encodes a G2 point in affine format into a byte slice with padded elements.
+/// Encodes a G2 point in affine format into byte slice with padded elements.
 pub(super) fn encode_g2_point(input: &blst_p2_affine) -> Bytes {
     let mut out = vec![0u8; G2_OUTPUT_LENGTH];
     fp_to_bytes(&mut out[..PADDED_FP_LENGTH], &input.x.fp[0]);
@@ -26,6 +27,33 @@ pub(super) fn encode_g2_point(input: &blst_p2_affine) -> Bytes {
     out.into()
 }
 
+/// Convert the following field elements from byte slices into a `blst_p2_affine` point.
+pub(super) fn decode_and_check_g2(
+    x1: &[u8; 48],
+    x2: &[u8; 48],
+    y1: &[u8; 48],
+    y2: &[u8; 48],
+) -> Result<blst_p2_affine, PrecompileError> {
+    Ok(blst_p2_affine {
+        x: check_canonical_fp2(x1, x2)?,
+        y: check_canonical_fp2(y1, y2)?,
+    })
+}
+
+/// Checks whether or not the input represents a canonical fp2 field element, returning the field
+/// element if successful.
+pub(super) fn check_canonical_fp2(
+    input_1: &[u8; 48],
+    input_2: &[u8; 48],
+) -> Result<blst_fp2, PrecompileError> {
+    let fp_1 = fp_from_bendian(input_1)?;
+    let fp_2 = fp_from_bendian(input_2)?;
+
+    let fp2 = blst_fp2 { fp: [fp_1, fp_2] };
+
+    Ok(fp2)
+}
+
 /// Extracts a G2 point in Affine format from a 256 byte slice representation.
 ///
 /// NOTE: This function will perform a G2 subgroup check if `subgroup_check` is set to `true`.
@@ -45,14 +73,7 @@ pub(super) fn extract_g2_input(
         input_fps[i] = remove_padding(&input[i * PADDED_FP_LENGTH..(i + 1) * PADDED_FP_LENGTH])?;
     }
 
-    let mut out = blst_p2_affine::default();
-    // SAFETY: items in fps have fixed length, out is a blst value.
-    unsafe {
-        blst_fp_from_bendian(&mut out.x.fp[0], input_fps[0].as_ptr());
-        blst_fp_from_bendian(&mut out.x.fp[1], input_fps[1].as_ptr());
-        blst_fp_from_bendian(&mut out.y.fp[0], input_fps[2].as_ptr());
-        blst_fp_from_bendian(&mut out.y.fp[1], input_fps[3].as_ptr());
-    }
+    let out = decode_and_check_g2(input_fps[0], input_fps[1], input_fps[2], input_fps[3])?;
 
     if subgroup_check {
         // NB: Subgroup checks

crates/precompile/src/bls12_381/g2_msm.rs

@@ -11,6 +11,7 @@ use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G2MSM precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(g2_msm));
+
 /// BLS12_G2MSM precompile address.
 pub const ADDRESS: u64 = 0x10;
 

crates/precompile/src/bls12_381/map_fp2_to_g2.rs

@@ -1,19 +1,19 @@
 use super::{
+    g2::check_canonical_fp2,
     g2::encode_g2_point,
     utils::{remove_padding, PADDED_FP2_LENGTH, PADDED_FP_LENGTH},
 };
 use crate::{u64_to_address, PrecompileWithAddress};
-use blst::{
-    blst_fp, blst_fp2, blst_fp_from_bendian, blst_map_to_g2, blst_p2, blst_p2_affine,
-    blst_p2_to_affine,
-};
+use blst::{blst_map_to_g2, blst_p2, blst_p2_affine, blst_p2_to_affine};
 use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_MAP_FP2_TO_G2 precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(map_fp2_to_g2));
+
 /// BLS12_MAP_FP2_TO_G2 precompile address.
 pub const ADDRESS: u64 = 0x13;
+
 /// Base gas fee for BLS12-381 map_fp2_to_g2 operation.
 const BASE_GAS_FEE: u64 = 75000;
 
@@ -35,16 +35,7 @@ pub(super) fn map_fp2_to_g2(input: &Bytes, gas_limit: u64) -> PrecompileResult {
 
     let input_p0_x = remove_padding(&input[..PADDED_FP_LENGTH])?;
     let input_p0_y = remove_padding(&input[PADDED_FP_LENGTH..PADDED_FP2_LENGTH])?;
-
-    let mut fp2 = blst_fp2::default();
-    let mut fp_x = blst_fp::default();
-    let mut fp_y = blst_fp::default();
-    // SAFETY: input_p0_x has fixed length, fp_x is a blst value.
-    unsafe { blst_fp_from_bendian(&mut fp_x, input_p0_x.as_ptr()) };
-    // SAFETY: input_p0_y has fixed length, fp_y is a blst value.
-    unsafe { blst_fp_from_bendian(&mut fp_y, input_p0_y.as_ptr()) };
-    fp2.fp[0] = fp_x;
-    fp2.fp[1] = fp_y;
+    let fp2 = check_canonical_fp2(input_p0_x, input_p0_y)?;
 
     let mut p = blst_p2::default();
     // SAFETY: p and fp2 are blst values.

crates/precompile/src/bls12_381/map_fp_to_g1.rs

@@ -1,18 +1,18 @@
 use super::{
     g1::encode_g1_point,
-    utils::{remove_padding, PADDED_FP_LENGTH},
+    utils::{fp_from_bendian, remove_padding, PADDED_FP_LENGTH},
 };
 use crate::{u64_to_address, PrecompileWithAddress};
-use blst::{
-    blst_fp, blst_fp_from_bendian, blst_map_to_g1, blst_p1, blst_p1_affine, blst_p1_to_affine,
-};
+use blst::{blst_map_to_g1, blst_p1, blst_p1_affine, blst_p1_to_affine};
 use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_MAP_FP_TO_G1 precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(map_fp_to_g1));
+
 /// BLS12_MAP_FP_TO_G1 precompile address.
 pub const ADDRESS: u64 = 0x12;
+
 /// Base gas fee for BLS12-381 map_fp_to_g1 operation.
 const MAP_FP_TO_G1_BASE: u64 = 5500;
 
@@ -32,11 +32,7 @@ pub(super) fn map_fp_to_g1(input: &Bytes, gas_limit: u64) -> PrecompileResult {
     }
 
     let input_p0 = remove_padding(input)?;
-
-    let mut fp = blst_fp::default();
-
-    // SAFETY: input_p0 has fixed length, fp is a blst value.
-    unsafe { blst_fp_from_bendian(&mut fp, input_p0.as_ptr()) };
+    let fp = fp_from_bendian(input_p0)?;
 
     let mut p = blst_p1::default();
     // SAFETY: p and fp are blst values.

crates/precompile/src/bls12_381/utils.rs

@@ -1,4 +1,8 @@
-use blst::{blst_bendian_from_fp, blst_fp, blst_scalar, blst_scalar_from_bendian};
+use core::cmp::Ordering;
+
+use blst::{
+    blst_bendian_from_fp, blst_fp, blst_fp_from_bendian, blst_scalar, blst_scalar_from_bendian,
+};
 use revm_primitives::PrecompileError;
 
 /// Number of bits used in the BLS12-381 curve finite field elements.
@@ -13,8 +17,14 @@ pub(super) const PADDED_FP2_LENGTH: usize = 128;
 pub(super) const PADDING_LENGTH: usize = 16;
 /// Scalar length.
 pub(super) const SCALAR_LENGTH: usize = 32;
+// Big-endian non-Montgomery form.
+pub(super) const MODULUS_REPR: [u8; 48] = [
+    0x1a, 0x01, 0x11, 0xea, 0x39, 0x7f, 0xe6, 0x9a, 0x4b, 0x1b, 0xa7, 0xb6, 0x43, 0x4b, 0xac, 0xd7,
+    0x64, 0x77, 0x4b, 0x84, 0xf3, 0x85, 0x12, 0xbf, 0x67, 0x30, 0xd2, 0xa0, 0xf6, 0xb0, 0xf6, 0x24,
+    0x1e, 0xab, 0xff, 0xfe, 0xb1, 0x53, 0xff, 0xff, 0xb9, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xab,
+];
 
-/// Encodes a single finite field element into a byte slice with padding.
+/// Encodes a single finite field element into byte slice with padding.
 pub(super) fn fp_to_bytes(out: &mut [u8], input: *const blst_fp) {
     if out.len() != PADDED_FP_LENGTH {
         return;
@@ -57,3 +67,32 @@ pub(super) fn extract_scalar_input(input: &[u8]) -> Result<blst_scalar, Precompi
 
     Ok(out)
 }
+
+/// Checks if the input is a valid big-endian representation of a field element.
+fn is_valid_be(input: &[u8; 48]) -> bool {
+    for (i, modul) in input.iter().zip(MODULUS_REPR.iter()) {
+        match i.cmp(modul) {
+            Ordering::Greater => return false,
+            Ordering::Less => return true,
+            Ordering::Equal => continue,
+        }
+    }
+    // false if matching the modulus
+    false
+}
+
+/// Checks whether or not the input represents a canonical field element, returning the field
+/// element if successful.
+pub(super) fn fp_from_bendian(input: &[u8; 48]) -> Result<blst_fp, PrecompileError> {
+    if !is_valid_be(input) {
+        return Err(PrecompileError::Other("non-canonical fp value".to_string()));
+    }
+    let mut fp = blst_fp::default();
+    // SAFETY: input has fixed length, and fp is a blst value.
+    unsafe {
+        // This performs the check for canonical field elements
+        blst_fp_from_bendian(&mut fp, input.as_ptr());
+    }
+
+    Ok(fp)
+}

crates/precompile/test-vectors/fail-add_G1_bls.json

@@ -0,0 +1,32 @@
+[
+    {
+        "Input": "",
+        "ExpectedError": "invalid input length",
+        "Name": "bls_g1add_empty_input"
+    },
+    {
+        "Input": "00000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb0000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e100000000000000000000000000000000112b98340eee2777cc3c14163dea3ec97977ac3dc5c70da32e6e87578f44912e902ccef9efe28d4a78b8999dfbca942600000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a21",
+        "ExpectedError": "invalid input length",
+        "Name": "bls_g1add_short_input"
+    },
+    {
+        "Input": "000000000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb0000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e100000000000000000000000000000000112b98340eee2777cc3c14163dea3ec97977ac3dc5c70da32e6e87578f44912e902ccef9efe28d4a78b8999dfbca942600000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a21",
+        "ExpectedError": "invalid input length",
+        "Name": "bls_g1add_large_input"
+    },
+    {
+        "Input": "0000000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb00000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a2100000000000000000000000000000000112b98340eee2777cc3c14163dea3ec97977ac3dc5c70da32e6e87578f44912e902ccef9efe28d4a78b8999dfbca942600000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a21",
+        "ExpectedError": "invalid point: not on curve",
+        "Name": "bls_g1add_point_not_on_curve"
+    },
+    {
+        "Input": "0000000000000000000000000000000031f2e5916b17be2e71b10b4292f558e727dfd7d48af9cbc5087f0ce00dcca27c8b01e83eaace1aefb539f00adb2271660000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e100000000000000000000000000000000112b98340eee2777cc3c14163dea3ec97977ac3dc5c70da32e6e87578f44912e902ccef9efe28d4a78b8999dfbca942600000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a21",
+        "ExpectedError": "invalid fp.Element encoding",
+        "Name": "bls_g2add_invalid_field_element"
+    },
+    {
+        "Input": "1000000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb0000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e100000000000000000000000000000000112b98340eee2777cc3c14163dea3ec97977ac3dc5c70da32e6e87578f44912e902ccef9efe28d4a78b8999dfbca942600000000000000000000000000000000186b28d92356c4dfec4b5201ad099dbdede3781f8998ddf929b4cd7756192185ca7b8f4ef7088f813270ac3d48868a21",
+        "ExpectedError": "invalid field element top bytes",
+        "Name": "bls_g1add_violate_top_bytes"
+    }
+]