Don't update if already an executable (#18252)

(cherry picked from commit 2fbedd834f)

# Conflicts:
#	runtime/src/message_processor.rs

Co-authored-by: Jack May <jack@solana.com>

programs/bpf/c/src/invoke/invoke.c

@@ -19,6 +19,8 @@ static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER = 12;
 static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 13;
 static const uint8_t TEST_WRITABLE_DEESCALATION_WRITABLE = 14;
 static const uint8_t TEST_NESTED_INVOKE_TOO_DEEP = 15;
+static const uint8_t TEST_EXECUTABLE_LAMPORTS = 16;
+static const uint8_t ADD_LAMPORTS = 17;
 
 static const int MINT_INDEX = 0;
 static const int ARGUMENT_INDEX = 1;
@@ -64,7 +66,7 @@ uint64_t do_nested_invokes(uint64_t num_nested_invokes,
 extern uint64_t entrypoint(const uint8_t *input) {
   sol_log("Invoke C program");
 
-  SolAccountInfo accounts[11];
+  SolAccountInfo accounts[12];
   SolParameters params = (SolParameters){.ka = accounts};
 
   if (!sol_deserialize(input, &params, SOL_ARRAY_SIZE(accounts))) {
@@ -534,6 +536,29 @@ extern uint64_t entrypoint(const uint8_t *input) {
     do_nested_invokes(5, accounts, params.ka_num);
     break;
   }
+  case TEST_EXECUTABLE_LAMPORTS: {
+      sol_log("Test executable lamports");
+      accounts[ARGUMENT_INDEX].executable = true;
+      *accounts[ARGUMENT_INDEX].lamports -= 1;
+      *accounts[DERIVED_KEY1_INDEX].lamports +=1;
+      SolAccountMeta arguments[] = {
+          {accounts[ARGUMENT_INDEX].key, true, false},
+          {accounts[DERIVED_KEY1_INDEX].key, true, false},
+      };
+      uint8_t data[] = {ADD_LAMPORTS, 0, 0, 0};
+      SolPubkey program_id;
+      sol_memcpy(&program_id, params.program_id, sizeof(SolPubkey));
+      const SolInstruction instruction = {&program_id,
+                                          arguments, SOL_ARRAY_SIZE(arguments),
+                                          data, SOL_ARRAY_SIZE(data)};
+      sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
+      *accounts[ARGUMENT_INDEX].lamports += 1;
+      break;
+  }
+  case ADD_LAMPORTS: {
+      *accounts[0].lamports += 1;
+      break;
+  }
 
   default:
     sol_panic();

programs/bpf/rust/invoke/src/lib.rs

@@ -31,6 +31,8 @@ const TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 12;
 const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13;
 const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14;
 const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15;
+const TEST_EXECUTABLE_LAMPORTS: u8 = 16;
+const ADD_LAMPORTS: u8 = 17;
 
 // const MINT_INDEX: usize = 0; // unused placeholder
 const ARGUMENT_INDEX: usize = 1;
@@ -619,6 +621,33 @@ fn process_instruction(
         TEST_NESTED_INVOKE_TOO_DEEP => {
             let _ = do_nested_invokes(5, accounts);
         }
+        TEST_EXECUTABLE_LAMPORTS => {
+            msg!("Test executable lamports");
+            let mut accounts = accounts.to_vec();
+
+            // set account to executable and subtract lamports
+            accounts[ARGUMENT_INDEX].executable = true;
+            **(*accounts[ARGUMENT_INDEX].lamports).borrow_mut() -= 1;
+            // add lamports to dest account
+            **(*accounts[DERIVED_KEY1_INDEX].lamports).borrow_mut() += 1;
+
+            let instruction = create_instruction(
+                *program_id,
+                &[
+                    (accounts[ARGUMENT_INDEX].key, true, false),
+                    (accounts[DERIVED_KEY1_INDEX].key, true, false),
+                ],
+                vec![ADD_LAMPORTS, 0, 0, 0],
+            );
+            let _ = invoke(&instruction, &accounts);
+
+            // reset executable account
+            **(*accounts[ARGUMENT_INDEX].lamports).borrow_mut() += 1;
+        }
+        ADD_LAMPORTS => {
+            // make sure the total balance is fine
+            **accounts[0].lamports.borrow_mut() += 1;
+        }
         _ => panic!(),
     }
 

programs/bpf/tests/programs.rs

@@ -755,6 +755,7 @@ fn test_program_bpf_invoke_sanity() {
     const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13;
     const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14;
     const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15;
+    const TEST_EXECUTABLE_LAMPORTS: u8 = 16;
 
     #[allow(dead_code)]
     #[derive(Debug)]
@@ -829,6 +830,7 @@ fn test_program_bpf_invoke_sanity() {
             AccountMeta::new_readonly(derived_key3, false),
             AccountMeta::new_readonly(solana_sdk::system_program::id(), false),
             AccountMeta::new(from_keypair.pubkey(), true),
+            AccountMeta::new_readonly(invoke_program_id, false),
         ];
 
         // success cases
@@ -851,7 +853,7 @@ fn test_program_bpf_invoke_sanity() {
             bank.last_blockhash(),
         );
         let (result, inner_instructions) = process_transaction_and_record_inner(&bank, tx);
-        assert!(result.is_ok());
+        assert_eq!(result, Ok(()));
 
         let invoked_programs: Vec<Pubkey> = inner_instructions[0]
             .iter()
@@ -934,7 +936,7 @@ fn test_program_bpf_invoke_sanity() {
                     .iter()
                     .map(|ix| message.account_keys[ix.program_id_index as usize].clone())
                     .collect();
-                assert_eq!(result.unwrap_err(), expected_error);
+                assert_eq!(result, Err(expected_error));
                 assert_eq!(invoked_programs, expected_invoked_programs);
             };
 
@@ -1022,6 +1024,12 @@ fn test_program_bpf_invoke_sanity() {
             ],
         );
 
+        do_invoke_failure_test_local(
+            TEST_EXECUTABLE_LAMPORTS,
+            TransactionError::InstructionError(0, InstructionError::ExecutableLamportChange),
+            &[invoke_program_id.clone()],
+        );
+
         // Check resulting state
 
         assert_eq!(43, bank.get_balance(&derived_key1));

runtime/src/message_processor.rs

@@ -228,6 +228,10 @@ impl PreAccount {
         self.account.borrow().lamports
     }
 
+    pub fn executable(&self) -> bool {
+        self.account.borrow().executable
+    }
+
     pub fn is_zeroed(buf: &[u8]) -> bool {
         const ZEROS_LEN: usize = 1024;
         static ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN];
@@ -1032,7 +1036,7 @@ impl MessageProcessor {
                         pre_account.verify(&program_id, is_writable, &rent, &account, timings)?;
                         pre_sum += u128::from(pre_account.lamports());
                         post_sum += u128::from(account.lamports);
-                        if is_writable && !account.executable {
+                        if is_writable && !pre_account.executable() {
                             pre_account.update(&account);
                         }
                         return Ok(());