program: add custom error codes for executable, ro writes

Author: buffalojoec

Cargo.lock

@@ -19,10 +19,13 @@ test-sbf = []
 [dependencies]
 bincode = "1.3.3"
 bytemuck = "1.14.1"
+num-derive = "0.4"
+num-traits = "0.2"
 serde = { version = "1.0.193", features = ["derive"] }
 solana-frozen-abi = { version = "2.0.1", optional = true }
 solana-frozen-abi-macro = { version = "2.0.1", optional = true }
 solana-program = "2.0.1"
+thiserror = "1.0.61"
 
 [dev-dependencies]
 mollusk-svm = { version = "0.0.5", features = ["fuzz"] }

program/Cargo.toml

@@ -1,8 +1,11 @@
 //! Program entrypoint
 
 use {
-    crate::processor,
-    solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey},
+    crate::{error::AddressLookupTableError, processor},
+    solana_program::{
+        account_info::AccountInfo, entrypoint::ProgramResult, program_error::PrintProgramError,
+        pubkey::Pubkey,
+    },
 };
 
 solana_program::entrypoint!(process_instruction);
@@ -11,5 +14,9 @@ fn process_instruction(
     accounts: &[AccountInfo],
     instruction_data: &[u8],
 ) -> ProgramResult {
-    processor::process(program_id, accounts, instruction_data)
+    if let Err(error) = processor::process(program_id, accounts, instruction_data) {
+        error.print::<AddressLookupTableError>();
+        return Err(error);
+    }
+    Ok(())
 }

program/src/entrypoint.rs

@@ -0,0 +1,40 @@
+//! Program error types.
+
+use {
+    num_derive::FromPrimitive,
+    solana_program::{
+        decode_error::DecodeError,
+        msg,
+        program_error::{PrintProgramError, ProgramError},
+    },
+    thiserror::Error,
+};
+
+/// Errors that can be returned by the Config program.
+#[derive(Error, Clone, Debug, Eq, PartialEq, FromPrimitive)]
+pub enum AddressLookupTableError {
+    /// Instruction changed executable account's data.
+    #[error("Instruction changed executable account's data")]
+    ExecutableDataModified = 10, // Avoid collisions with SystemError from CPI.
+    /// Instruction modified data of a read-only account.
+    #[error("Instruction modified data of a read-only account")]
+    ReadonlyDataModified,
+}
+
+impl PrintProgramError for AddressLookupTableError {
+    fn print<E>(&self) {
+        msg!(&self.to_string());
+    }
+}
+
+impl From<AddressLookupTableError> for ProgramError {
+    fn from(e: AddressLookupTableError) -> Self {
+        ProgramError::Custom(e as u32)
+    }
+}
+
+impl<T> DecodeError<T> for AddressLookupTableError {
+    fn type_of() -> &'static str {
+        "AddressLookupTableError"
+    }
+}

program/src/error.rs

@@ -3,6 +3,7 @@
 
 #[cfg(all(target_os = "solana", feature = "bpf-entrypoint"))]
 mod entrypoint;
+pub mod error;
 pub mod instruction;
 pub mod processor;
 pub mod state;

program/src/lib.rs

@@ -3,6 +3,7 @@
 use {
     crate::{
         check_id,
+        error::AddressLookupTableError,
         instruction::AddressLookupTableInstruction,
         state::{
             AddressLookupTable, ProgramState, LOOKUP_TABLE_MAX_ADDRESSES, LOOKUP_TABLE_META_SIZE,
@@ -145,6 +146,8 @@ fn process_create_lookup_table(
         )?;
     }
 
+    // [Core BPF]: No need to check `is_executable` or `is_writable` here,
+    // since they will be checked by System.
     invoke_signed(
         &system_instruction::allocate(lookup_table_info.key, lookup_table_data_len as u64),
         &[lookup_table_info.clone()],
@@ -205,6 +208,10 @@ fn process_freeze_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo]) ->
         lookup_table.meta
     };
 
+    // [Core BPF]: No need to check `is_executable` or `is_writable` here,
+    // since only non-frozen lookup tables can be frozen (never a same-data
+    // write).
+
     lookup_table_meta.authority = None;
     AddressLookupTable::overwrite_meta_data(
         &mut lookup_table_info.try_borrow_mut_data()?[..],
@@ -297,6 +304,36 @@ fn process_extend_lookup_table(
         )
     };
 
+    // [Core BPF]:
+    // When a builtin program attempts to write to an executable or read-only
+    // account, it will be immediately rejected by the `TransactionContext`.
+    // For more information, see https://github.com/solana-program/config/pull/21.
+    //
+    // However, in the case of the Address Lookup Table program's
+    // `ExtendLookupTable` instruction, since the processor rejects any
+    // zero-length "new keys" vectors, and will gladly append the same keys
+    // again to the table, the issue here is slightly different than the linked
+    // PR.
+    //
+    // The builtin version of the Address Lookup Table program will throw
+    // when it attempts to overwrite the metadata, while the BPF version will
+    // continue. In the case where an executable or read-only lookup table
+    // account is provided, and some other requirement below is violated
+    // (ie. no payer or system program accounts provided, payer is not a
+    // signer, payer has insufficent balance, etc.), the BPF version will throw
+    // based on one of those violations, rather than throwing immediately when
+    // it encounters the executable or read-only lookup table account.
+    //
+    // As a result, this placement of these mocked out `InstructionError`
+    // variants ensures maximum backwards compatibility with the builtin
+    // version.
+    if lookup_table_info.executable {
+        return Err(AddressLookupTableError::ExecutableDataModified.into());
+    }
+    if !lookup_table_info.is_writable {
+        return Err(AddressLookupTableError::ReadonlyDataModified.into());
+    }
+
     AddressLookupTable::overwrite_meta_data(
         &mut lookup_table_info.try_borrow_mut_data()?[..],
         lookup_table_meta,
@@ -374,6 +411,11 @@ fn process_deactivate_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo]
     };
 
     let clock = <Clock as Sysvar>::get()?;
+
+    // [Core BPF]: No need to check `is_executable` or `is_writable` here,
+    // since only non-deactivated lookup tables can be deactivated (never a
+    // same-data write).
+
     lookup_table_meta.deactivation_slot = clock.slot;
 
     AddressLookupTable::overwrite_meta_data(
@@ -444,6 +486,10 @@ fn process_close_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo]) ->
         }?;
     }
 
+    // [Core BPF]: No need to check `is_executable` or `is_writable` here,
+    // since only non-closed lookup tables can be deactivated. Deserialization
+    // would fail earlier in the processor.
+
     let new_recipient_lamports = lookup_table_info
         .lamports()
         .checked_add(recipient_info.lamports())

program/src/processor.rs

@@ -10,6 +10,7 @@ use {
         Mollusk,
     },
     solana_address_lookup_table_program::{
+        error::AddressLookupTableError,
         instruction::extend_lookup_table,
         state::{AddressLookupTable, LookupTableMeta},
     },
@@ -355,3 +356,77 @@ fn test_extend_prepaid_lookup_table_without_payer() {
         },
     );
 }
+
+// Backwards compatibility test case.
+#[test]
+fn test_extend_executable() {
+    let mollusk = setup();
+
+    let payer = Pubkey::new_unique();
+    let authority = Pubkey::new_unique();
+
+    let initialized_table = new_address_lookup_table(Some(authority), 0);
+
+    let lookup_table_address = Pubkey::new_unique();
+    let mut lookup_table_account = lookup_table_account(initialized_table);
+
+    // Make the lookup table account executable.
+    lookup_table_account.set_executable(true);
+
+    let new_addresses = vec![Pubkey::new_unique()];
+    let instruction =
+        extend_lookup_table(lookup_table_address, authority, Some(payer), new_addresses);
+
+    mollusk.process_and_validate_instruction(
+        &instruction,
+        &[
+            (lookup_table_address, lookup_table_account),
+            (authority, AccountSharedData::default()),
+            (
+                payer,
+                AccountSharedData::new(100_000_000, 0, &system_program::id()),
+            ),
+            keyed_account_for_system_program(),
+        ],
+        &[Check::err(ProgramError::Custom(
+            AddressLookupTableError::ExecutableDataModified as u32,
+        ))],
+    );
+}
+
+// Backwards compatibility test case.
+#[test]
+fn test_extend_readonly() {
+    let mollusk = setup();
+
+    let payer = Pubkey::new_unique();
+    let authority = Pubkey::new_unique();
+
+    let initialized_table = new_address_lookup_table(Some(authority), 0);
+
+    let lookup_table_address = Pubkey::new_unique();
+    let lookup_table_account = lookup_table_account(initialized_table);
+
+    let new_addresses = vec![Pubkey::new_unique()];
+    let mut instruction =
+        extend_lookup_table(lookup_table_address, authority, Some(payer), new_addresses);
+
+    // Make the lookup table account read-only.
+    instruction.accounts[0].is_writable = false;
+
+    mollusk.process_and_validate_instruction(
+        &instruction,
+        &[
+            (lookup_table_address, lookup_table_account),
+            (authority, AccountSharedData::default()),
+            (
+                payer,
+                AccountSharedData::new(100_000_000, 0, &system_program::id()),
+            ),
+            keyed_account_for_system_program(),
+        ],
+        &[Check::err(ProgramError::Custom(
+            AddressLookupTableError::ReadonlyDataModified as u32,
+        ))],
+    );
+}