Dont call precompiled programs (#19930)

sdk/src/ed25519_instruction.rs

@@ -170,6 +170,15 @@ fn get_data_slice<'a>(
 #[cfg(test)]
 pub mod test {
     use super::*;
+    use crate::{
+        ed25519_instruction::new_ed25519_instruction,
+        feature_set::FeatureSet,
+        hash::Hash,
+        signature::{Keypair, Signer},
+        transaction::Transaction,
+    };
+    use rand::{thread_rng, Rng};
+    use std::sync::Arc;
 
     fn test_case(
         num_signatures: u16,
@@ -322,4 +331,34 @@ pub mod test {
             Err(PrecompileError::InvalidDataOffsets)
         );
     }
+
+    #[test]
+    fn test_ed25519() {
+        solana_logger::setup();
+
+        let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng());
+        let message_arr = b"hello";
+        let mut instruction = new_ed25519_instruction(&privkey, message_arr);
+        let mint_keypair = Keypair::new();
+        let feature_set = Arc::new(FeatureSet::all_enabled());
+
+        let tx = Transaction::new_signed_with_payer(
+            &[instruction.clone()],
+            Some(&mint_keypair.pubkey()),
+            &[&mint_keypair],
+            Hash::default(),
+        );
+
+        assert!(tx.verify_precompiles(&feature_set).is_ok());
+
+        let index = thread_rng().gen_range(0, instruction.data.len());
+        instruction.data[index] = instruction.data[index].wrapping_add(12);
+        let tx = Transaction::new_signed_with_payer(
+            &[instruction],
+            Some(&mint_keypair.pubkey()),
+            &[&mint_keypair],
+            Hash::default(),
+        );
+        assert!(tx.verify_precompiles(&feature_set).is_err());
+    }
 }

sdk/src/feature_set.rs

@@ -223,6 +223,12 @@ pub mod do_support_realloc {
     solana_sdk::declare_id!("75m6ysz33AfLA5DDEzWM1obBrnPQRSsdVQ2nRmc8Vuu1");
 }
 
+// Note: when this feature is cleaned up, also remove the secp256k1 program from
+// the list of builtins and remove its files from /programs
+pub mod prevent_calling_precompiles_as_programs {
+    solana_sdk::declare_id!("4ApgRX3ud6p7LNMJmsuaAcZY5HWctGPr5obAsjB3A54d");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -274,6 +280,7 @@ lazy_static! {
         (sol_log_data_syscall_enabled::id(), "enable sol_log_data syscall"),
         (stakes_remove_delegation_if_inactive::id(), "remove delegations from stakes cache when inactive"),
         (do_support_realloc::id(), "support account data reallocation"),
+        (prevent_calling_precompiles_as_programs::id(), "Prevent calling precompiles as programs"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()

sdk/src/precompiles.rs

@@ -5,7 +5,9 @@
 use {
     crate::{
         decode_error::DecodeError,
-        feature_set::{ed25519_program_enabled, FeatureSet},
+        feature_set::{
+            ed25519_program_enabled, prevent_calling_precompiles_as_programs, FeatureSet,
+        },
         instruction::CompiledInstruction,
         pubkey::Pubkey,
     },
@@ -38,7 +40,7 @@ impl<T> DecodeError<T> for PrecompileError {
 pub type Verify = fn(&[u8], &[&[u8]], &Arc<FeatureSet>) -> std::result::Result<(), PrecompileError>;
 
 /// Information on a precompiled program
-struct Precompile {
+pub struct Precompile {
     /// Program id
     pub program_id: Pubkey,
     /// Feature to enable on, `None` indicates always enabled
@@ -56,8 +58,13 @@ impl Precompile {
         }
     }
     /// Check if a program id is this precompiled program
-    pub fn check_id(&self, program_id: &Pubkey, feature_set: &Arc<FeatureSet>) -> bool {
-        self.feature.map_or(true, |f| feature_set.is_active(&f)) && self.program_id == *program_id
+    pub fn check_id<F>(&self, program_id: &Pubkey, is_enabled: F) -> bool
+    where
+        F: Fn(&Pubkey) -> bool,
+    {
+        self.feature
+            .map_or(true, |ref feature_id| is_enabled(feature_id))
+            && self.program_id == *program_id
     }
     /// Verify this precompiled program
     pub fn verify(
@@ -75,7 +82,7 @@ lazy_static! {
     static ref PRECOMPILES: Vec<Precompile> = vec![
         Precompile::new(
             crate::secp256k1_program::id(),
-            None,
+            Some(prevent_calling_precompiles_as_programs::id()),
             crate::secp256k1_instruction::verify,
         ),
         Precompile::new(
@@ -87,10 +94,17 @@ lazy_static! {
 }
 
 /// Check if a program is a precompiled program
-pub fn is_precompile(program_id: &Pubkey, feature_set: &Arc<FeatureSet>) -> bool {
+pub fn is_precompile<F>(program_id: &Pubkey, is_enabled: F) -> bool
+where
+    F: Fn(&Pubkey) -> bool,
+{
     PRECOMPILES
         .iter()
-        .any(|precompile| precompile.check_id(program_id, feature_set))
+        .any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id)))
+}
+
+pub fn get_precompiles<'a>() -> &'a [Precompile] {
+    &PRECOMPILES
 }
 
 /// Check that a program is precompiled and if so verify it
@@ -101,7 +115,7 @@ pub fn verify_if_precompile(
     feature_set: &Arc<FeatureSet>,
 ) -> Result<(), PrecompileError> {
     for precompile in PRECOMPILES.iter() {
-        if precompile.check_id(program_id, feature_set) {
+        if precompile.check_id(program_id, |feature_id| feature_set.is_active(feature_id)) {
             let instruction_datas: Vec<_> = all_instructions
                 .iter()
                 .map(|instruction| instruction.data.as_ref())

sdk/src/secp256k1_instruction.rs

@@ -212,6 +212,17 @@ fn get_data_slice<'a>(
 #[cfg(test)]
 pub mod test {
     use super::*;
+    use crate::{
+        feature_set,
+        hash::Hash,
+        secp256k1_instruction::{
+            new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE,
+        },
+        signature::{Keypair, Signer},
+        transaction::Transaction,
+    };
+    use rand::{thread_rng, Rng};
+    use std::sync::Arc;
 
     fn test_case(
         num_signatures: u8,
@@ -390,4 +401,46 @@ pub mod test {
             Err(PrecompileError::InvalidInstructionDataSize)
         );
     }
+
+    #[test]
+    fn test_secp256k1() {
+        solana_logger::setup();
+        let offsets = SecpSignatureOffsets::default();
+        assert_eq!(
+            bincode::serialized_size(&offsets).unwrap() as usize,
+            SIGNATURE_OFFSETS_SERIALIZED_SIZE
+        );
+
+        let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng());
+        let message_arr = b"hello";
+        let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr);
+        let mint_keypair = Keypair::new();
+        let mut feature_set = feature_set::FeatureSet::all_enabled();
+        feature_set
+            .active
+            .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id());
+        feature_set
+            .inactive
+            .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id());
+        let feature_set = Arc::new(feature_set);
+
+        let tx = Transaction::new_signed_with_payer(
+            &[secp_instruction.clone()],
+            Some(&mint_keypair.pubkey()),
+            &[&mint_keypair],
+            Hash::default(),
+        );
+
+        assert!(tx.verify_precompiles(&feature_set).is_ok());
+
+        let index = thread_rng().gen_range(0, secp_instruction.data.len());
+        secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12);
+        let tx = Transaction::new_signed_with_payer(
+            &[secp_instruction],
+            Some(&mint_keypair.pubkey()),
+            &[&mint_keypair],
+            Hash::default(),
+        );
+        assert!(tx.verify_precompiles(&feature_set).is_err());
+    }
 }