Fix - Revert and feature gate incorrect error message in BPF loader (#…

…30748)

* Revert to old behavior.

* Adds feature gate.

programs/bpf_loader/src/lib.rs

@@ -45,7 +45,8 @@ use {
             check_slice_translation_size, delay_visibility_of_program_deployment,
             disable_deploy_of_alloc_free_syscall, enable_bpf_loader_extend_program_ix,
             enable_bpf_loader_set_authority_checked_ix, enable_program_redeployment_cooldown,
-            limit_max_instruction_trace_length, round_up_heap_size, FeatureSet,
+            limit_max_instruction_trace_length, remove_bpf_loader_incorrect_program_id,
+            round_up_heap_size, FeatureSet,
         },
         instruction::{AccountMeta, InstructionError},
         loader_instruction::LoaderInstruction,
@@ -440,20 +441,88 @@ fn process_instruction_common(
     let log_collector = invoke_context.get_log_collector();
     let transaction_context = &invoke_context.transaction_context;
     let instruction_context = transaction_context.get_current_instruction_context()?;
+
+    if !invoke_context
+        .feature_set
+        .is_active(&remove_bpf_loader_incorrect_program_id::id())
+    {
+        fn get_index_in_transaction(
+            instruction_context: &InstructionContext,
+            index_in_instruction: IndexOfAccount,
+        ) -> Result<IndexOfAccount, InstructionError> {
+            if index_in_instruction < instruction_context.get_number_of_program_accounts() {
+                instruction_context
+                    .get_index_of_program_account_in_transaction(index_in_instruction)
+            } else {
+                instruction_context.get_index_of_instruction_account_in_transaction(
+                    index_in_instruction
+                        .saturating_sub(instruction_context.get_number_of_program_accounts()),
+                )
+            }
+        }
+
+        fn try_borrow_account<'a>(
+            transaction_context: &'a TransactionContext,
+            instruction_context: &'a InstructionContext,
+            index_in_instruction: IndexOfAccount,
+        ) -> Result<BorrowedAccount<'a>, InstructionError> {
+            if index_in_instruction < instruction_context.get_number_of_program_accounts() {
+                instruction_context
+                    .try_borrow_program_account(transaction_context, index_in_instruction)
+            } else {
+                instruction_context.try_borrow_instruction_account(
+                    transaction_context,
+                    index_in_instruction
+                        .saturating_sub(instruction_context.get_number_of_program_accounts()),
+                )
+            }
+        }
+
+        let first_instruction_account = {
+            let borrowed_root_account =
+                instruction_context.try_borrow_program_account(transaction_context, 0)?;
+            let owner_id = borrowed_root_account.get_owner();
+            if native_loader::check_id(owner_id) {
+                1
+            } else {
+                0
+            }
+        };
+        let first_account_key = transaction_context.get_key_of_account_at_index(
+            get_index_in_transaction(instruction_context, first_instruction_account)?,
+        )?;
+        let second_account_key = get_index_in_transaction(
+            instruction_context,
+            first_instruction_account.saturating_add(1),
+        )
+        .and_then(|index_in_transaction| {
+            transaction_context.get_key_of_account_at_index(index_in_transaction)
+        });
+        let program_id = instruction_context.get_last_program_key(transaction_context)?;
+        if first_account_key == program_id
+            || second_account_key
+                .map(|key| key == program_id)
+                .unwrap_or(false)
+        {
+        } else {
+            let first_account = try_borrow_account(
+                transaction_context,
+                instruction_context,
+                first_instruction_account,
+            )?;
+            if first_account.is_executable() {
+                ic_logger_msg!(log_collector, "BPF loader is executable");
+                return Err(InstructionError::IncorrectProgramId);
+            }
+        }
+    }
+
     let program_account =
         instruction_context.try_borrow_last_program_account(transaction_context)?;
 
     // Program Management Instruction
     if native_loader::check_id(program_account.get_owner()) {
         drop(program_account);
-        if instruction_context
-            .try_borrow_instruction_account(transaction_context, 0)
-            .map(|account| account.is_executable())
-            .unwrap_or(false)
-        {
-            ic_logger_msg!(log_collector, "BPF loader is executable");
-            return Err(InstructionError::IncorrectProgramId);
-        }
         let program_id = instruction_context.get_last_program_key(transaction_context)?;
         return if bpf_loader_upgradeable::check_id(program_id) {
             process_loader_upgradeable_instruction(invoke_context, use_jit)

programs/sbf/tests/programs.rs

@@ -2188,6 +2188,9 @@ fn test_program_sbf_disguised_as_sbf_loader() {
         let mut bank = Bank::new_for_tests(&genesis_config);
         let (name, id, entrypoint) = solana_bpf_loader_program!();
         bank.add_builtin(&name, &id, entrypoint);
+        bank.deactivate_feature(
+            &solana_sdk::feature_set::remove_bpf_loader_incorrect_program_id::id(),
+        );
         let bank_client = BankClient::new(bank);
 
         let program_id = load_program(&bank_client, &bpf_loader::id(), &mint_keypair, program);

sdk/src/feature_set.rs

@@ -630,6 +630,10 @@ pub mod round_up_heap_size {
     solana_sdk::declare_id!("CE2et8pqgyQMP2mQRg3CgvX8nJBKUArMu3wfiQiQKY1y");
 }
 
+pub mod remove_bpf_loader_incorrect_program_id {
+    solana_sdk::declare_id!("2HmTkCj9tXuPE4ueHzdD7jPeMf9JGCoZh5AsyoATiWEe");
+}
+
 pub mod vote_state_add_vote_latency {
     solana_sdk::declare_id!("7axKe5BTYBDD87ftzWbk5DfzWMGyRvqmWTduuo22Yaqy");
 }
@@ -786,6 +790,7 @@ lazy_static! {
         (add_set_tx_loaded_accounts_data_size_instruction::id(), "add compute budget instruction for setting account data size per transaction #30366"),
         (switch_to_new_elf_parser::id(), "switch to new ELF parser #30497"),
         (round_up_heap_size::id(), "round up heap size when calculating heap cost #30679"),
+        (remove_bpf_loader_incorrect_program_id::id(), "stop incorrectly throwing IncorrectProgramId in bpf_loader #30747"),
         (vote_state_add_vote_latency::id(), "replace Lockout with LandedVote (including vote latency) in vote state"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]