Feature: Explicitly limit `TransactionContext::instruction_trace_capacity` (#27938)

* Renames instruction_stack_capacity => instruction_stack_capacity.

* Replaces number_of_instructions_at_transaction_level by instruction_trace_capacity.

* Adds MaxInstructionTraceLengthExceeded.

* Adjusts TransactionContext::new() parameter.

* Adds feature gate limit_max_instruction_trace_length.

* Adds test_max_instruction_trace_length().
This commit is contained in:
Alexander Meißner 2022-09-26 10:47:16 +02:00 committed by GitHub
parent de7a5f2c68
commit 71aee4fcaf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 106 additions and 29 deletions

View File

@ -85,6 +85,7 @@ log_u64_units: 100,
create_program address units: 1500,
invoke_units: 1000,
max_invoke_depth: 4,
max_instruction_trace_length: 64,
max_call_depth: 64,
stack_frame_size: 4096,
log_pubkey_units: 100,

View File

@ -37,6 +37,8 @@ pub struct ComputeBudget {
pub invoke_units: u64,
/// Maximum cross-program invocation depth allowed
pub max_invoke_depth: usize,
/// Maximum cross-program invocation and instructions per transaction
pub max_instruction_trace_length: usize,
/// Base number of compute units consumed to call SHA256
pub sha256_base_cost: u64,
/// Incremental number of units consumed by SHA256 (based on bytes)
@ -100,6 +102,7 @@ impl ComputeBudget {
create_program_address_units: 1500,
invoke_units: 1000,
max_invoke_depth: 4,
max_instruction_trace_length: 64,
sha256_base_cost: 85,
sha256_byte_cost: 1,
sha256_max_slices: 20_000,

View File

@ -1022,11 +1022,12 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
}];
let preparation =
prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices);
let compute_budget = ComputeBudget::default();
let mut transaction_context = TransactionContext::new(
preparation.transaction_accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
compute_budget.max_invoke_depth.saturating_add(1),
compute_budget.max_instruction_trace_length,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
@ -1057,11 +1058,12 @@ pub fn mock_process_instruction(
preparation
.transaction_accounts
.push((*loader_id, processor_account));
let compute_budget = ComputeBudget::default();
let mut transaction_context = TransactionContext::new(
preparation.transaction_accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
compute_budget.max_invoke_depth.saturating_add(1),
compute_budget.max_instruction_trace_length,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
@ -1284,7 +1286,7 @@ mod tests {
accounts,
Some(Rent::default()),
ComputeBudget::default().max_invoke_depth,
1,
MAX_DEPTH,
);
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
@ -1309,6 +1311,21 @@ mod tests {
assert!(depth_reached < MAX_DEPTH);
}
#[test]
fn test_max_instruction_trace_length() {
const MAX_INSTRUCTIONS: usize = 8;
let mut transaction_context =
TransactionContext::new(Vec::new(), Some(Rent::default()), 1, MAX_INSTRUCTIONS);
for _ in 0..MAX_INSTRUCTIONS {
transaction_context.push().unwrap();
transaction_context.pop().unwrap();
}
assert_eq!(
transaction_context.push(),
Err(InstructionError::MaxInstructionTraceLengthExceeded)
);
}
#[test]
fn test_process_instruction() {
let callee_program_id = solana_sdk::pubkey::new_rand();
@ -1345,7 +1362,7 @@ mod tests {
})
.collect::<Vec<_>>();
let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 2, 9);
TransactionContext::new(accounts, Some(Rent::default()), 2, 18);
let mut invoke_context =
InvokeContext::new_mock(&mut transaction_context, builtin_programs);
@ -1436,7 +1453,7 @@ mod tests {
let accounts = vec![(solana_sdk::pubkey::new_rand(), AccountSharedData::default())];
let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 1, 3);
TransactionContext::new(accounts, Some(Rent::default()), 1, 1);
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
invoke_context.compute_budget =
ComputeBudget::new(compute_budget::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64);

View File

@ -44,12 +44,14 @@ use {
cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
disable_deploy_of_alloc_free_syscall, disable_deprecated_loader,
enable_bpf_loader_extend_program_ix, error_on_syscall_bpf_function_hash_collisions,
reject_callx_r10,
limit_max_instruction_trace_length, reject_callx_r10,
},
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
program_error::MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED,
program_error::{
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED, MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED,
},
program_utils::limited_deserialize,
pubkey::Pubkey,
saturating_add_assign,
@ -1362,14 +1364,20 @@ impl Executor for BpfExecutor {
}
match result {
Ok(status) if status != SUCCESS => {
let error: InstructionError = if status
let error: InstructionError = if (status
== MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&cap_accounts_data_allocations_per_transaction::id())
.is_active(&cap_accounts_data_allocations_per_transaction::id()))
|| (status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&limit_max_instruction_trace_length::id()))
{
// Until the cap_accounts_data_allocations_per_transaction feature is
// enabled, map the MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED error to InvalidError
// enabled, map the `MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED` error to `InvalidError`.
// Until the limit_max_instruction_trace_length feature is
// enabled, map the `MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED` error to `InvalidError`.
InstructionError::InvalidError
} else {
status.into()

View File

@ -3870,8 +3870,10 @@ mod tests {
)
})
.collect::<Vec<_>>();
let mut transaction_context = TransactionContext::new(transaction_accounts, None, 4, 1);
for (index_in_trace, stack_height) in [1, 2, 3, 2, 2, 3, 4, 3].into_iter().enumerate() {
let instruction_trace = [1, 2, 3, 2, 2, 3, 4, 3];
let mut transaction_context =
TransactionContext::new(transaction_accounts, None, 4, instruction_trace.len());
for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
while stack_height <= transaction_context.get_instruction_context_stack_height() {
transaction_context.pop().unwrap();
}

View File

@ -285,7 +285,7 @@ impl RentDebits {
}
pub type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "7FSSacrCi7vf2QZFm3Ui9JqTii4U6h1XWYD3LKSuVwV8")]
#[frozen_abi(digest = "A7T7XohiSoo8FGoCPTsaXAYYugXTkoYnBjQAdBgYHH85")]
pub type BankSlotDelta = SlotDelta<Result<()>>;
// Eager rent collection repeats in cyclic manner.
@ -4363,7 +4363,14 @@ impl Bank {
None
},
compute_budget.max_invoke_depth.saturating_add(1),
tx.message().instructions().len(),
if self
.feature_set
.is_active(&feature_set::limit_max_instruction_trace_length::id())
{
compute_budget.max_instruction_trace_length
} else {
std::usize::MAX
},
);
if self
.feature_set
@ -18774,7 +18781,6 @@ pub(crate) mod tests {
sol_to_lamports(1.),
bank.last_blockhash(),
);
let number_of_instructions_at_transaction_level = tx.message().instructions.len();
let num_accounts = tx.message().account_keys.len();
let sanitized_tx = SanitizedTransaction::try_from_legacy_transaction(tx).unwrap();
let mut error_counters = TransactionErrorMetrics::default();
@ -18797,7 +18803,7 @@ pub(crate) mod tests {
loaded_txs[0].0.as_ref().unwrap().accounts.clone(),
Some(Rent::default()),
compute_budget.max_invoke_depth.saturating_add(1),
number_of_instructions_at_transaction_level,
compute_budget.max_instruction_trace_length,
);
assert_eq!(
@ -18945,8 +18951,10 @@ pub(crate) mod tests {
#[test]
fn test_inner_instructions_list_from_instruction_trace() {
let mut transaction_context = TransactionContext::new(vec![], None, 3, 3);
for (index_in_trace, stack_height) in [1, 2, 1, 1, 2, 3, 2].into_iter().enumerate() {
let instruction_trace = [1, 2, 1, 1, 2, 3, 2];
let mut transaction_context =
TransactionContext::new(vec![], None, 3, instruction_trace.len());
for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
while stack_height <= transaction_context.get_instruction_context_stack_height() {
transaction_context.pop().unwrap();
}

View File

@ -334,7 +334,7 @@ mod test {
},
];
let mut transaction_context =
TransactionContext::new(accounts, Some(Rent::default()), 1, 2);
TransactionContext::new(accounts, Some(Rent::default()), 1, 1);
let mut $invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
};
}

View File

@ -256,6 +256,10 @@ pub enum InstructionError {
/// Max accounts exceeded
#[error("Max accounts exceeded")]
MaxAccountsExceeded,
/// Max instruction trace length exceeded
#[error("Max instruction trace length exceeded")]
MaxInstructionTraceLengthExceeded,
// Note: For any new error added here an equivalent ProgramError and its
// conversions must also be added
}

View File

@ -55,6 +55,8 @@ pub enum ProgramError {
MaxAccountsDataAllocationsExceeded,
#[error("Account data reallocation was invalid")]
InvalidRealloc,
#[error("Instruction trace length exceeded the maximum allowed per transaction")]
MaxInstructionTraceLengthExceeded,
}
pub trait PrintProgramError {
@ -97,6 +99,9 @@ impl PrintProgramError for ProgramError {
msg!("Error: MaxAccountsDataAllocationsExceeded")
}
Self::InvalidRealloc => msg!("Error: InvalidRealloc"),
Self::MaxInstructionTraceLengthExceeded => {
msg!("Error: MaxInstructionTraceLengthExceeded")
}
}
}
}
@ -129,6 +134,7 @@ pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17);
pub const ILLEGAL_OWNER: u64 = to_builtin!(18);
pub const MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED: u64 = to_builtin!(19);
pub const INVALID_ACCOUNT_DATA_REALLOC: u64 = to_builtin!(20);
pub const MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED: u64 = to_builtin!(21);
// Warning: Any new program errors added here must also be:
// - Added to the below conversions
// - Added as an equivalent to InstructionError
@ -159,6 +165,9 @@ impl From<ProgramError> for u64 {
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
}
ProgramError::InvalidRealloc => INVALID_ACCOUNT_DATA_REALLOC,
ProgramError::MaxInstructionTraceLengthExceeded => {
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
}
ProgramError::Custom(error) => {
if error == 0 {
CUSTOM_ZERO
@ -193,6 +202,7 @@ impl From<u64> for ProgramError {
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED => Self::MaxInstructionTraceLengthExceeded,
_ => Self::Custom(error as u32),
}
}
@ -225,6 +235,9 @@ impl TryFrom<InstructionError> for ProgramError {
Ok(Self::MaxAccountsDataAllocationsExceeded)
}
Self::Error::InvalidRealloc => Ok(Self::InvalidRealloc),
Self::Error::MaxInstructionTraceLengthExceeded => {
Ok(Self::MaxInstructionTraceLengthExceeded)
}
_ => Err(error),
}
}
@ -257,6 +270,7 @@ where
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED => Self::MaxInstructionTraceLengthExceeded,
_ => {
// A valid custom error has no bits set in the upper 32
if error >> BUILTIN_BIT_SHIFT == 0 {

View File

@ -526,6 +526,10 @@ pub mod increase_tx_account_lock_limit {
solana_sdk::declare_id!("9LZdXeKGeBV6hRLdxS1rHbHoEUsKqesCC2ZAPTPKJAbK");
}
pub mod limit_max_instruction_trace_length {
solana_sdk::declare_id!("GQALDaC48fEhZGWRj9iL5Q889emJKcj3aCvHF7VCbbF4");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -652,6 +656,7 @@ lazy_static! {
(epoch_accounts_hash::id(), "enable epoch accounts hash calculation #27539"),
(remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"),
(increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
(limit_max_instruction_trace_length::id(), "limit max instruction trace length"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

View File

@ -113,7 +113,8 @@ pub struct TransactionContext {
accounts: Pin<Box<[RefCell<AccountSharedData>]>>,
#[cfg(not(target_os = "solana"))]
account_touched_flags: RefCell<Pin<Box<[bool]>>>,
instruction_context_capacity: usize,
instruction_stack_capacity: usize,
instruction_trace_capacity: usize,
instruction_stack: Vec<usize>,
instruction_trace: Vec<InstructionContext>,
return_data: TransactionReturnData,
@ -130,8 +131,8 @@ impl TransactionContext {
pub fn new(
transaction_accounts: Vec<TransactionAccount>,
rent: Option<Rent>,
instruction_context_capacity: usize,
_number_of_instructions_at_transaction_level: usize,
instruction_stack_capacity: usize,
instruction_trace_capacity: usize,
) -> Self {
let (account_keys, accounts): (Vec<Pubkey>, Vec<RefCell<AccountSharedData>>) =
transaction_accounts
@ -143,8 +144,9 @@ impl TransactionContext {
account_keys: Pin::new(account_keys.into_boxed_slice()),
accounts: Pin::new(accounts.into_boxed_slice()),
account_touched_flags: RefCell::new(Pin::new(account_touched_flags.into_boxed_slice())),
instruction_context_capacity,
instruction_stack: Vec::with_capacity(instruction_context_capacity),
instruction_stack_capacity,
instruction_trace_capacity,
instruction_stack: Vec::with_capacity(instruction_stack_capacity),
instruction_trace: vec![InstructionContext::default()],
return_data: TransactionReturnData::default(),
accounts_resize_delta: RefCell::new(0),
@ -213,6 +215,11 @@ impl TransactionContext {
.map(|index| index as IndexOfAccount)
}
/// Gets the max length of the InstructionContext trace
pub fn get_instruction_trace_capacity(&self) -> usize {
self.instruction_trace_capacity
}
/// Returns the instruction trace length.
///
/// Not counting the last empty InstructionContext which is always pre-reserved for the next instruction.
@ -246,8 +253,8 @@ impl TransactionContext {
}
/// Gets the max height of the InstructionContext stack
pub fn get_instruction_context_capacity(&self) -> usize {
self.instruction_context_capacity
pub fn get_instruction_stack_capacity(&self) -> usize {
self.instruction_stack_capacity
}
/// Gets instruction stack height, top-level instructions are height
@ -307,8 +314,11 @@ impl TransactionContext {
callee_instruction_accounts_lamport_sum;
}
let index_in_trace = self.get_instruction_trace_length();
if index_in_trace >= self.instruction_trace_capacity {
return Err(InstructionError::MaxInstructionTraceLengthExceeded);
}
self.instruction_trace.push(InstructionContext::default());
if nesting_level >= self.instruction_context_capacity {
if nesting_level >= self.instruction_stack_capacity {
return Err(InstructionError::CallDepth);
}
self.instruction_stack.push(index_in_trace);

View File

@ -122,6 +122,7 @@ enum InstructionErrorType {
ILLEGAL_OWNER = 49;
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED = 50;
MAX_ACCOUNTS_EXCEEDED = 51;
MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED = 52;
}
message UnixTimestamp {

View File

@ -712,6 +712,7 @@ impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
49 => InstructionError::IllegalOwner,
50 => InstructionError::MaxAccountsDataAllocationsExceeded,
51 => InstructionError::MaxAccountsExceeded,
52 => InstructionError::MaxInstructionTraceLengthExceeded,
_ => return Err("Invalid InstructionError"),
};
@ -1031,6 +1032,9 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
InstructionError::MaxAccountsExceeded => {
tx_by_addr::InstructionErrorType::MaxAccountsExceeded
}
InstructionError::MaxInstructionTraceLengthExceeded => {
tx_by_addr::InstructionErrorType::MaxInstructionTraceLengthExceeded
}
} as i32,
custom: match instruction_error {
InstructionError::Custom(custom) => {