fix(precompile): blst dangling pointers, cleanup (bluealloy#1391)

* fix(precompile): blst dangling pointers, cleanup * chore: correct error message
taikoxyz · May 10, 2024 · ebbd76a · ebbd76a
1 parent a4b466a
commit ebbd76a
Show file tree

Hide file tree

Showing 13 changed files with 111 additions and 193 deletions.
diff --git a/crates/precompile/src/bls12_381/g1.rs b/crates/precompile/src/bls12_381/g1.rs
@@ -1,8 +1,7 @@
+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};
 use revm_primitives::{Bytes, PrecompileError};
 
-use super::utils::{fp_to_bytes, remove_padding, PADDED_FP_LENGTH};
-
 /// 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.
@@ -20,10 +19,10 @@ pub(super) fn encode_g1_point(input: *const blst_p1_affine) -> Bytes {
 }
 
 /// Extracts a G1 point in Affine format from a 128 byte slice representation.
-pub(super) fn extract_g1_input(input: &[u8]) -> Result<*const blst_p1_affine, PrecompileError> {
+pub(super) fn extract_g1_input(input: &[u8]) -> Result<blst_p1_affine, PrecompileError> {
     if input.len() != G1_INPUT_ITEM_LENGTH {
         return Err(PrecompileError::Other(format!(
-            "Input should be {G1_INPUT_ITEM_LENGTH} bits, was {}",
+            "Input should be {G1_INPUT_ITEM_LENGTH} bytes, was {}",
             input.len()
         )));
     }
@@ -39,10 +38,8 @@ pub(super) fn extract_g1_input(input: &[u8]) -> Result<*const blst_p1_affine, Pr
     }
 
     // SAFETY: out is a blst value.
-    unsafe {
-        if !blst_p1_affine_in_g1(&out) {
-            return Err(PrecompileError::Other("Element not in G1".to_string()));
-        }
+    if unsafe { !blst_p1_affine_in_g1(&out) } {
+        return Err(PrecompileError::Other("Element not in G1".to_string()));
     }
-    Ok(&mut out as *const _)
+    Ok(out)
 }
diff --git a/crates/precompile/src/bls12_381/g1_add.rs b/crates/precompile/src/bls12_381/g1_add.rs
@@ -1,12 +1,10 @@
+use super::g1::{encode_g1_point, extract_g1_input, G1_INPUT_ITEM_LENGTH};
+use crate::{u64_to_address, PrecompileWithAddress};
 use blst::{
     blst_p1, blst_p1_add_or_double_affine, blst_p1_affine, blst_p1_from_affine, blst_p1_to_affine,
 };
 use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
-use crate::{u64_to_address, PrecompileWithAddress};
-
-use super::g1::{encode_g1_point, extract_g1_input, G1_INPUT_ITEM_LENGTH};
-
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G1ADD precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(g1_add));
@@ -30,31 +28,25 @@ fn g1_add(input: &Bytes, gas_limit: u64) -> PrecompileResult {
 
     if input.len() != INPUT_LENGTH {
         return Err(PrecompileError::Other(format!(
-            "G1ADD Input should be {INPUT_LENGTH} bits, was {}",
+            "G1ADD input should be {INPUT_LENGTH} bytes, was {}",
             input.len()
         )));
     }
 
-    let a_aff = extract_g1_input(&input[..G1_INPUT_ITEM_LENGTH])?;
-    let b_aff = extract_g1_input(&input[G1_INPUT_ITEM_LENGTH..])?;
+    let a_aff = &extract_g1_input(&input[..G1_INPUT_ITEM_LENGTH])?;
+    let b_aff = &extract_g1_input(&input[G1_INPUT_ITEM_LENGTH..])?;
 
     let mut b = blst_p1::default();
     // SAFETY: b and b_aff are blst values.
-    unsafe {
-        blst_p1_from_affine(&mut b, b_aff);
-    }
+    unsafe { blst_p1_from_affine(&mut b, b_aff) };
 
     let mut p = blst_p1::default();
     // SAFETY: p, b and a_aff are blst values.
-    unsafe {
-        blst_p1_add_or_double_affine(&mut p, &b, a_aff);
-    }
+    unsafe { blst_p1_add_or_double_affine(&mut p, &b, a_aff) };
 
     let mut p_aff = blst_p1_affine::default();
     // SAFETY: p_aff and p are blst values.
-    unsafe {
-        blst_p1_to_affine(&mut p_aff, &p);
-    }
+    unsafe { blst_p1_to_affine(&mut p_aff, &p) };
 
     let out = encode_g1_point(&p_aff);
     Ok((BASE_GAS_FEE, out))

diff --git a/crates/precompile/src/bls12_381/g1_msm.rs b/crates/precompile/src/bls12_381/g1_msm.rs
@@ -1,14 +1,12 @@
-use blst::{blst_p1, blst_p1_affine, blst_p1_from_affine, blst_p1_to_affine, p1_affines};
-use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
-
-use crate::{u64_to_address, PrecompileWithAddress};
-
 use super::{
     g1::{encode_g1_point, extract_g1_input, G1_INPUT_ITEM_LENGTH},
     g1_mul,
     msm::msm_required_gas,
     utils::{extract_scalar_input, NBITS, SCALAR_LENGTH},
 };
+use crate::{u64_to_address, PrecompileWithAddress};
+use blst::{blst_p1, blst_p1_affine, blst_p1_from_affine, blst_p1_to_affine, p1_affines};
+use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G1MSM precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
@@ -43,15 +41,12 @@ fn g1_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult {
     let mut g1_points: Vec<blst_p1> = Vec::with_capacity(k);
     let mut scalars: Vec<u8> = Vec::with_capacity(k * SCALAR_LENGTH);
     for i in 0..k {
-        let p0_aff = extract_g1_input(
+        let p0_aff = &extract_g1_input(
             &input[i * g1_mul::INPUT_LENGTH..i * g1_mul::INPUT_LENGTH + G1_INPUT_ITEM_LENGTH],
         )?;
         let mut p0 = blst_p1::default();
         // SAFETY: p0 and p0_aff are blst values.
-        unsafe {
-            blst_p1_from_affine(&mut p0, p0_aff);
-        }
-
+        unsafe { blst_p1_from_affine(&mut p0, p0_aff) };
         g1_points.push(p0);
 
         scalars.extend_from_slice(
@@ -68,9 +63,7 @@ fn g1_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult {
 
     let mut multiexp_aff = blst_p1_affine::default();
     // SAFETY: multiexp_aff and multiexp are blst values.
-    unsafe {
-        blst_p1_to_affine(&mut multiexp_aff, &multiexp);
-    }
+    unsafe { blst_p1_to_affine(&mut multiexp_aff, &multiexp) };
 
     let out = encode_g1_point(&multiexp_aff);
     Ok((required_gas, out))

diff --git a/crates/precompile/src/bls12_381/g1_mul.rs b/crates/precompile/src/bls12_381/g1_mul.rs
@@ -1,12 +1,10 @@
-use blst::{blst_p1, blst_p1_affine, blst_p1_from_affine, blst_p1_mult, blst_p1_to_affine};
-use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
-
-use crate::{u64_to_address, PrecompileWithAddress};
-
 use super::{
     g1::{encode_g1_point, extract_g1_input, G1_INPUT_ITEM_LENGTH},
     utils::{extract_scalar_input, NBITS},
 };
+use crate::{u64_to_address, PrecompileWithAddress};
+use blst::{blst_p1, blst_p1_affine, blst_p1_from_affine, blst_p1_mult, blst_p1_to_affine};
+use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G1MUL precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
@@ -31,30 +29,24 @@ pub fn g1_mul(input: &Bytes, gas_limit: u64) -> PrecompileResult {
     }
     if input.len() != INPUT_LENGTH {
         return Err(PrecompileError::Other(format!(
-            "G1MUL Input should be {INPUT_LENGTH} bits, was {}",
+            "G1MUL input should be {INPUT_LENGTH} bytes, was {}",
             input.len()
         )));
     }
 
-    let p0_aff = extract_g1_input(&input[..G1_INPUT_ITEM_LENGTH])?;
+    let p0_aff = &extract_g1_input(&input[..G1_INPUT_ITEM_LENGTH])?;
     let mut p0 = blst_p1::default();
     // SAFETY: p0 and p0_aff are blst values.
-    unsafe {
-        blst_p1_from_affine(&mut p0, p0_aff);
-    }
+    unsafe { blst_p1_from_affine(&mut p0, p0_aff) };
 
     let input_scalar0 = extract_scalar_input(&input[G1_INPUT_ITEM_LENGTH..])?;
 
     let mut p = blst_p1::default();
     // SAFETY: input_scalar0.b has fixed size, p and p0 are blst values.
-    unsafe {
-        blst_p1_mult(&mut p, &p0, input_scalar0.b.as_ptr(), NBITS);
-    }
+    unsafe { blst_p1_mult(&mut p, &p0, input_scalar0.b.as_ptr(), NBITS) };
     let mut p_aff = blst_p1_affine::default();
     // SAFETY: p_aff and p are blst values.
-    unsafe {
-        blst_p1_to_affine(&mut p_aff, &p);
-    }
+    unsafe { blst_p1_to_affine(&mut p_aff, &p) };
 
     let out = encode_g1_point(&p_aff);
     Ok((BASE_GAS_FEE, out))

diff --git a/crates/precompile/src/bls12_381/g2.rs b/crates/precompile/src/bls12_381/g2.rs
@@ -1,45 +1,41 @@
+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};
 use revm_primitives::{Bytes, PrecompileError};
 
-use super::utils::{fp_to_bytes, remove_padding, FP_LENGTH, PADDED_FP_LENGTH};
-
 /// 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.
-pub(super) fn encode_g2_point(input: *const blst_p2_affine) -> Bytes {
+pub(super) fn encode_g2_point(input: &blst_p2_affine) -> Bytes {
     let mut out = vec![0u8; G2_OUTPUT_LENGTH];
-    // SAFETY: out comes from fixed length array, input is a blst value.
-    unsafe {
-        fp_to_bytes(&mut out[..PADDED_FP_LENGTH], &(*input).x.fp[0]);
-        fp_to_bytes(
-            &mut out[PADDED_FP_LENGTH..2 * PADDED_FP_LENGTH],
-            &(*input).x.fp[1],
-        );
-        fp_to_bytes(
-            &mut out[2 * PADDED_FP_LENGTH..3 * PADDED_FP_LENGTH],
-            &(*input).y.fp[0],
-        );
-        fp_to_bytes(
-            &mut out[3 * PADDED_FP_LENGTH..4 * PADDED_FP_LENGTH],
-            &(*input).y.fp[1],
-        );
-    }
+    fp_to_bytes(&mut out[..PADDED_FP_LENGTH], &input.x.fp[0]);
+    fp_to_bytes(
+        &mut out[PADDED_FP_LENGTH..2 * PADDED_FP_LENGTH],
+        &input.x.fp[1],
+    );
+    fp_to_bytes(
+        &mut out[2 * PADDED_FP_LENGTH..3 * PADDED_FP_LENGTH],
+        &input.y.fp[0],
+    );
+    fp_to_bytes(
+        &mut out[3 * PADDED_FP_LENGTH..4 * PADDED_FP_LENGTH],
+        &input.y.fp[1],
+    );
     out.into()
 }
 
 /// Extracts a G2 point in Affine format from a 256 byte slice representation.
-pub(super) fn extract_g2_input(input: &[u8]) -> Result<*const blst_p2_affine, PrecompileError> {
+pub(super) fn extract_g2_input(input: &[u8]) -> Result<blst_p2_affine, PrecompileError> {
     if input.len() != G2_INPUT_ITEM_LENGTH {
         return Err(PrecompileError::Other(format!(
-            "Input should be {G2_INPUT_ITEM_LENGTH} bits, was {}",
+            "Input should be {G2_INPUT_ITEM_LENGTH} bytes, was {}",
             input.len()
         )));
     }
 
-    let mut input_fps: [[u8; FP_LENGTH]; 4] = [[0; FP_LENGTH]; 4];
+    let mut input_fps: [&[u8; FP_LENGTH]; 4] = [&[0; FP_LENGTH]; 4];
     for i in 0..4 {
         input_fps[i] = remove_padding(&input[i * PADDED_FP_LENGTH..(i + 1) * PADDED_FP_LENGTH])?;
     }
@@ -54,11 +50,9 @@ pub(super) fn extract_g2_input(input: &[u8]) -> Result<*const blst_p2_affine, Pr
     }
 
     // SAFETY: out is a blst value.
-    unsafe {
-        if !blst_p2_affine_in_g2(&out) {
-            return Err(PrecompileError::Other("Element not in G2".to_string()));
-        }
+    if unsafe { !blst_p2_affine_in_g2(&out) } {
+        return Err(PrecompileError::Other("Element not in G2".to_string()));
     }
 
-    Ok(&mut out as *const _)
+    Ok(out)
 }
diff --git a/crates/precompile/src/bls12_381/g2_add.rs b/crates/precompile/src/bls12_381/g2_add.rs
@@ -1,12 +1,10 @@
+use super::g2::{encode_g2_point, extract_g2_input, G2_INPUT_ITEM_LENGTH};
+use crate::{u64_to_address, PrecompileWithAddress};
 use blst::{
     blst_p2, blst_p2_add_or_double_affine, blst_p2_affine, blst_p2_from_affine, blst_p2_to_affine,
 };
 use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
-use crate::{u64_to_address, PrecompileWithAddress};
-
-use super::g2::{encode_g2_point, extract_g2_input, G2_INPUT_ITEM_LENGTH};
-
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G2ADD precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
     PrecompileWithAddress(u64_to_address(ADDRESS), Precompile::Standard(g2_add));
@@ -31,31 +29,25 @@ fn g2_add(input: &Bytes, gas_limit: u64) -> PrecompileResult {
 
     if input.len() != INPUT_LENGTH {
         return Err(PrecompileError::Other(format!(
-            "G2ADD Input should be {INPUT_LENGTH} bits, was {}",
+            "G2ADD input should be {INPUT_LENGTH} bytes, was {}",
             input.len()
         )));
     }
 
-    let a_aff = extract_g2_input(&input[..G2_INPUT_ITEM_LENGTH])?;
-    let b_aff = extract_g2_input(&input[G2_INPUT_ITEM_LENGTH..])?;
+    let a_aff = &extract_g2_input(&input[..G2_INPUT_ITEM_LENGTH])?;
+    let b_aff = &extract_g2_input(&input[G2_INPUT_ITEM_LENGTH..])?;
 
     let mut b = blst_p2::default();
     // SAFETY: b and b_aff are blst values.
-    unsafe {
-        blst_p2_from_affine(&mut b, b_aff);
-    }
+    unsafe { blst_p2_from_affine(&mut b, b_aff) };
 
     let mut p = blst_p2::default();
     // SAFETY: p, b and a_aff are blst values.
-    unsafe {
-        blst_p2_add_or_double_affine(&mut p, &b, a_aff);
-    }
+    unsafe { blst_p2_add_or_double_affine(&mut p, &b, a_aff) };
 
     let mut p_aff = blst_p2_affine::default();
     // SAFETY: p_aff and p are blst values.
-    unsafe {
-        blst_p2_to_affine(&mut p_aff, &p);
-    }
+    unsafe { blst_p2_to_affine(&mut p_aff, &p) };
 
     let out = encode_g2_point(&p_aff);
     Ok((BASE_GAS_FEE, out))

diff --git a/crates/precompile/src/bls12_381/g2_msm.rs b/crates/precompile/src/bls12_381/g2_msm.rs
@@ -1,14 +1,12 @@
-use blst::{blst_p2, blst_p2_affine, blst_p2_from_affine, blst_p2_to_affine, p2_affines};
-use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
-
-use crate::{u64_to_address, PrecompileWithAddress};
-
 use super::{
     g2::{encode_g2_point, extract_g2_input, G2_INPUT_ITEM_LENGTH},
     g2_mul,
     msm::msm_required_gas,
     utils::{extract_scalar_input, NBITS, SCALAR_LENGTH},
 };
+use crate::{u64_to_address, PrecompileWithAddress};
+use blst::{blst_p2, blst_p2_affine, blst_p2_from_affine, blst_p2_to_affine, p2_affines};
+use revm_primitives::{Bytes, Precompile, PrecompileError, PrecompileResult};
 
 /// [EIP-2537](https://eips.ethereum.org/EIPS/eip-2537#specification) BLS12_G2MSM precompile.
 pub const PRECOMPILE: PrecompileWithAddress =
@@ -43,14 +41,12 @@ fn g2_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult {
     let mut g2_points: Vec<blst_p2> = Vec::with_capacity(k);
     let mut scalars: Vec<u8> = Vec::with_capacity(k * SCALAR_LENGTH);
     for i in 0..k {
-        let p0_aff = extract_g2_input(
+        let p0_aff = &extract_g2_input(
             &input[i * g2_mul::INPUT_LENGTH..i * g2_mul::INPUT_LENGTH + G2_INPUT_ITEM_LENGTH],
         )?;
         let mut p0 = blst_p2::default();
         // SAFETY: p0 and p0_aff are blst values.
-        unsafe {
-            blst_p2_from_affine(&mut p0, p0_aff);
-        }
+        unsafe { blst_p2_from_affine(&mut p0, p0_aff) };
 
         g2_points.push(p0);
 
@@ -68,9 +64,7 @@ fn g2_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult {
 
     let mut multiexp_aff = blst_p2_affine::default();
     // SAFETY: multiexp_aff and multiexp are blst values.
-    unsafe {
-        blst_p2_to_affine(&mut multiexp_aff, &multiexp);
-    }
+    unsafe { blst_p2_to_affine(&mut multiexp_aff, &multiexp) };
 
     let out = encode_g2_point(&multiexp_aff);
     Ok((required_gas, out))