Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HackerOne-2354265] Restrict number of imports, program depth, and call depth. #2352

Merged
merged 15 commits into from
Feb 15, 2024
5 changes: 5 additions & 0 deletions console/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ pub trait Network:
/// The maximum number of outputs per transition.
const MAX_OUTPUTS: usize = 16;

/// The maximum program depth.
const MAX_PROGRAM_DEPTH: usize = 1024;
/// The maximum number of imports.
const MAX_IMPORTS: usize = 2048;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide some context on why these depths and imports are where they're at?

Based on our design, I believe we're constraining other parameters already at the 32 mark. It would make sense to constraint both of these parameters alongside the 32 mark.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MAX_PROGRAM_DEPTH was decided based off of Ethereum's MAX_CALL_DEPTH.
I went with this as a healthy upper bound. I see no issue with restricting MAX_PROGRAM_DEPTH to 32.

MAX_IMPORTS should be larger because a user may want to create a program that reads the mappings of a large number of other programs.

Copy link
Contributor Author

@d0cd d0cd Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah actually, suppose that program1 reads program2 reads ... reads program32
If we program0 to reads program1, then we need a greater MAX_PROGRAM_DEPTH.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline, waiting for a change to 64 and 64


/// The state root type.
type StateRoot: Bech32ID<Field<Self>>;
/// The block hash type.
Expand Down
2 changes: 1 addition & 1 deletion ledger/block/src/transaction/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use super::*;

impl<N: Network> Transaction<N> {
/// The maximum number of transitions allowed in a transaction.
const MAX_TRANSITIONS: usize = usize::pow(2, TRANSACTION_DEPTH as u32);
pub const MAX_TRANSITIONS: usize = usize::pow(2, TRANSACTION_DEPTH as u32);

/// Returns the transaction root, by computing the root for a Merkle tree of the transition IDs.
pub fn to_root(&self) -> Result<Field<N>> {
Expand Down
35 changes: 34 additions & 1 deletion synthesizer/process/src/stack/helpers/initialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ impl<N: Network> Stack<N> {
universal_srs: process.universal_srs().clone(),
proving_keys: Default::default(),
verifying_keys: Default::default(),
number_of_calls: Default::default(),
program_depth: 0,
};

// Add all of the imports into the stack.
Expand All @@ -39,17 +41,49 @@ impl<N: Network> Stack<N> {
let external_stack = process.get_stack(import)?;
// Add the external stack to the stack.
stack.insert_external_stack(external_stack.clone())?;
// Update the program depth, checking that it does not exceed the maximum call depth.
stack.program_depth = std::cmp::max(stack.program_depth, external_stack.program_depth() + 1);
ensure!(
stack.program_depth <= N::MAX_PROGRAM_DEPTH,
"Program depth exceeds the maximum allowed call depth"
);
}
// Add the program closures to the stack.
for closure in program.closures().values() {
// Add the closure to the stack.
stack.insert_closure(closure)?;
}

// Add the program functions to the stack.
for function in program.functions().values() {
// Add the function to the stack.
stack.insert_function(function)?;
// Determine the number of calls for the function.
let mut num_calls = 1;
for instruction in function.instructions() {
if let Instruction::Call(call) = instruction {
// Determine if this is a function call.
if call.is_function_call(&stack)? {
// Increment by the number of calls.
num_calls += match call.operator() {
CallOperator::Locator(locator) => stack
.get_external_stack(locator.program_id())?
.get_number_of_calls(locator.resource())?,
CallOperator::Resource(resource) => stack.get_number_of_calls(resource)?,
};
}
}
}
// Check that the number of calls does not exceed the maximum.
// Note that one transition is reserved for the fee.
ensure!(
num_calls < ledger_block::Transaction::<N>::MAX_TRANSITIONS,
"Number of calls exceeds the maximum allowed number of transitions"
);
// Add the number of calls to the stack.
stack.number_of_calls.insert(*function.name(), num_calls);
howardwu marked this conversation as resolved.
Show resolved Hide resolved
}

// Return the stack.
Ok(stack)
}
Expand Down Expand Up @@ -109,7 +143,6 @@ impl<N: Network> Stack<N> {
// Add the finalize name and finalize types to the stack.
self.finalize_types.insert(*name, finalize_types);
}

// Return success.
Ok(())
}
Expand Down
31 changes: 14 additions & 17 deletions synthesizer/process/src/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ pub struct Stack<N: Network> {
proving_keys: Arc<RwLock<IndexMap<Identifier<N>, ProvingKey<N>>>>,
/// The mapping of function name to verifying key.
verifying_keys: Arc<RwLock<IndexMap<Identifier<N>, VerifyingKey<N>>>>,
/// The mapping of function names to the number of calls.
number_of_calls: IndexMap<Identifier<N>, usize>,
/// The program depth.
program_depth: usize,
}

impl<N: Network> Stack<N> {
Expand Down Expand Up @@ -220,6 +224,12 @@ impl<N: Network> StackProgram<N> for Stack<N> {
&self.program
}

/// Returns the program depth.
#[inline]
fn program_depth(&self) -> usize {
howardwu marked this conversation as resolved.
Show resolved Hide resolved
self.program_depth
}

/// Returns the program ID.
#[inline]
fn program_id(&self) -> &ProgramID<N> {
Expand Down Expand Up @@ -279,23 +289,10 @@ impl<N: Network> StackProgram<N> for Stack<N> {
/// Returns the expected number of calls for the given function name.
#[inline]
fn get_number_of_calls(&self, function_name: &Identifier<N>) -> Result<usize> {
// Determine the number of calls for this function (including the function itself).
let mut num_calls = 1;
for instruction in self.get_function(function_name)?.instructions() {
if let Instruction::Call(call) = instruction {
// Determine if this is a function call.
if call.is_function_call(self)? {
// Increment by the number of calls.
num_calls += match call.operator() {
CallOperator::Locator(locator) => {
self.get_external_stack(locator.program_id())?.get_number_of_calls(locator.resource())?
}
CallOperator::Resource(resource) => self.get_number_of_calls(resource)?,
};
}
}
}
Ok(num_calls)
self.number_of_calls
.get(function_name)
.copied()
.ok_or_else(|| anyhow!("Function '{function_name}' does not exist"))
}

/// Returns a value for the given value type.
Expand Down
93 changes: 92 additions & 1 deletion synthesizer/process/src/tests/test_execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use ledger_store::{
FinalizeStorage,
FinalizeStore,
};
use synthesizer_program::{FinalizeGlobalState, FinalizeStoreTrait, Program};
use synthesizer_program::{FinalizeGlobalState, FinalizeStoreTrait, Program, StackProgram};
use synthesizer_snark::UniversalSRS;

use indexmap::IndexMap;
Expand Down Expand Up @@ -2483,3 +2483,94 @@ function {function_name}:
assert_ne!(execution_1.peek().unwrap().id(), execution_2.peek().unwrap().id());
assert_ne!(execution_1.to_execution_id().unwrap(), execution_2.to_execution_id().unwrap());
}

#[test]
fn test_long_import_chain() {
// Initialize a new program.
let program = Program::<CurrentNetwork>::from_str(
r"
program test0.aleo;
function c:",
)
.unwrap();

// Construct the process.
let mut process = crate::test_helpers::sample_process(&program);

// Add 1024 programs to the process.
for i in 1..=1024 {
println!("Adding program {i}");
// Initialize a new program.
let program = Program::from_str(&format!(
"
import test{}.aleo;
program test{}.aleo;
function c:",
i - 1,
i
))
.unwrap();
// Add the program to the process.
process.add_program(&program).unwrap();
}

// Add the 1025th program to the process, which should fail.
let program = Program::from_str(
r"
import test1024.aleo;
program test1025.aleo;
function c:",
)
.unwrap();
let result = process.add_program(&program);
assert!(result.is_err());
}

#[test]
fn test_long_import_chain_with_calls() {
// Initialize a new program.
let program = Program::<CurrentNetwork>::from_str(
r"
program test0.aleo;
function c:",
)
.unwrap();

// Construct the process.
let mut process = crate::test_helpers::sample_process(&program);

// Check that the number of calls is correct.
for i in 1..31 {
println!("Adding program {}", i);
// Initialize a new program.
let program = Program::from_str(&format!(
"
import test{}.aleo;
program test{}.aleo;
function c:
call test{}.aleo/c;",
i - 1,
i,
i - 1
))
.unwrap();
// Add the program to the process.
process.add_program(&program).unwrap();
// Check that the number of calls is correct.
let stack = process.get_stack(program.id()).unwrap();
let number_of_calls = stack.get_number_of_calls(program.functions().into_iter().next().unwrap().0).unwrap();
assert_eq!(number_of_calls, i + 1);
}

// Check that an additional level of import will fail.
let program = Program::from_str(
r"
import test30.aleo;
program test31.aleo;
function c:
call test30.aleo/c;",
)
.unwrap();
let result = process.add_program(&program);
assert!(result.is_err())
}
3 changes: 3 additions & 0 deletions synthesizer/program/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ impl<N: Network, Instruction: InstructionTrait<N>, Command: CommandTrait<N>> Pro
// Retrieve the imported program name.
let import_name = *import.name();

// Ensure that the number of imports is within the allowed range.
ensure!(self.imports.len() < N::MAX_IMPORTS, "Program exceeds the maximum number of imports");

// Ensure the import name is new.
ensure!(self.is_unique_name(&import_name), "'{import_name}' is already in use.");
// Ensure the import name is not a reserved opcode.
Expand Down
3 changes: 3 additions & 0 deletions synthesizer/program/src/traits/stack_and_registers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ pub trait StackProgram<N: Network> {
/// Returns the program.
fn program(&self) -> &Program<N>;

/// Returns the program depth.
fn program_depth(&self) -> usize;
howardwu marked this conversation as resolved.
Show resolved Hide resolved

/// Returns the program ID.
fn program_id(&self) -> &ProgramID<N>;

Expand Down