Skip to content

Commit

Permalink
cherry-pick: improved value depth checks in the VM (#8488)
Browse files Browse the repository at this point in the history
  • Loading branch information
vgao1996 authored Jun 2, 2023
1 parent d5d8786 commit 47a0391
Show file tree
Hide file tree
Showing 11 changed files with 327 additions and 35 deletions.
1 change: 1 addition & 0 deletions aptos-move/aptos-vm/src/move_vm_ext/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl MoveVmExt {
paranoid_type_checks: crate::AptosVM::get_paranoid_checks(),
enable_invariant_violation_check_in_swap_loc,
type_size_limit,
max_value_nest_depth: Some(128),
},
)?,
chain_id,
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/framework/move-stdlib/tests/bcs_tests.move
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ module std::bcs_tests {
bcs::to_bytes(&box127(true));
}

/* Deactivated because we now limit the depth of values you could create inside the VM
#[test]
#[expected_failure(abort_code = 453, location = std::bcs)]
fun encode_129() {
bcs::to_bytes(&Box { x: box127(true) });
}
*/
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module std::event_tests {
}

#[test(s = @0x42)]
#[expected_failure(abort_code = 0, location = std::event)]
#[expected_failure] // VM_MAX_VALUE_DEPTH_REACHED
fun test_event_129(s: signer) acquires MyEvent {
event_129(&s);
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-stdlib/tests/bcs_tests.move
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ module std::bcs_tests {
}

#[test]
#[expected_failure(abort_code = 453, location = std::bcs)]
#[expected_failure] // VM_MAX_VALUE_DEPTH_REACHED
fun encode_129() {
bcs::to_bytes(&Box { x: box127(true) });
}
Expand Down
5 changes: 5 additions & 0 deletions third_party/move/move-vm/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
use move_binary_format::file_format_common::VERSION_MAX;
use move_bytecode_verifier::VerifierConfig;

pub const DEFAULT_MAX_VALUE_NEST_DEPTH: u64 = 128;

/// Dynamic config options for the Move VM.
pub struct VMConfig {
pub verifier: VerifierConfig,
Expand All @@ -14,6 +16,8 @@ pub struct VMConfig {
// When this flag is set to true, MoveVM will check invariant violation in swap_loc
pub enable_invariant_violation_check_in_swap_loc: bool,
pub type_size_limit: bool,
/// Maximum value nest depth for structs
pub max_value_nest_depth: Option<u64>,
}

impl Default for VMConfig {
Expand All @@ -24,6 +28,7 @@ impl Default for VMConfig {
paranoid_type_checks: false,
enable_invariant_violation_check_in_swap_loc: true,
type_size_limit: false,
max_value_nest_depth: Some(DEFAULT_MAX_VALUE_NEST_DEPTH),
}
}
}
Expand Down
90 changes: 90 additions & 0 deletions third_party/move/move-vm/runtime/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,91 @@ impl CallStack {
}
}

fn check_depth_of_type(resolver: &Resolver, ty: &Type) -> PartialVMResult<()> {
// Start at 1 since we always call this right before we add a new node to the value's depth.
let max_depth = match resolver.loader().vm_config().max_value_nest_depth {
Some(max_depth) => max_depth,
None => return Ok(()),
};
check_depth_of_type_impl(resolver, ty, max_depth, 1)?;
Ok(())
}

fn check_depth_of_type_impl(
resolver: &Resolver,
ty: &Type,
max_depth: u64,
depth: u64,
) -> PartialVMResult<u64> {
macro_rules! check_depth {
($additional_depth:expr) => {{
let new_depth = depth.saturating_add($additional_depth);
if new_depth > max_depth {
return Err(PartialVMError::new(StatusCode::VM_MAX_VALUE_DEPTH_REACHED));
} else {
new_depth
}
}};
}

// Calculate depth of the type itself
let ty_depth = match ty {
Type::Bool
| Type::U8
| Type::U16
| Type::U32
| Type::U64
| Type::U128
| Type::U256
| Type::Address
| Type::Signer => check_depth!(0),
// Even though this is recursive this is OK since the depth of this recursion is
// bounded by the depth of the type arguments, which we have already checked.
Type::Reference(ty) | Type::MutableReference(ty) | Type::Vector(ty) => {
check_depth_of_type_impl(resolver, ty, max_depth, check_depth!(1))?
},
Type::Struct(si) => {
let struct_type = resolver.loader().get_struct_type(*si).ok_or_else(|| {
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
.with_message("Struct Definition not resolved".to_string())
})?;
check_depth!(struct_type
.depth
.as_ref()
.ok_or_else(|| { PartialVMError::new(StatusCode::VM_MAX_VALUE_DEPTH_REACHED) })?
.solve(&[]))
},
// NB: substitution must be performed before calling this function
Type::StructInstantiation(si, ty_args) => {
// Calculate depth of all type arguments, and make sure they themselves are not too deep.
let ty_arg_depths = ty_args
.iter()
.map(|ty| {
// Ty args should be fully resolved and not need any type arguments
check_depth_of_type_impl(resolver, ty, max_depth, check_depth!(0))
})
.collect::<PartialVMResult<Vec<_>>>()?;
let struct_type = resolver.loader().get_struct_type(*si).ok_or_else(|| {
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
.with_message("Struct Definition not resolved".to_string())
})?;
check_depth!(struct_type
.depth
.as_ref()
.ok_or_else(|| { PartialVMError::new(StatusCode::VM_MAX_VALUE_DEPTH_REACHED) })?
.solve(&ty_arg_depths))
},
Type::TyParam(_) => {
return Err(
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
.with_message("Type parameter should be fully resolved".to_string()),
)
},
};

Ok(ty_depth)
}

/// A `Frame` is the execution context for a function. It holds the locals of the function and
/// the function itself.
// #[derive(Debug)]
Expand Down Expand Up @@ -1870,6 +1955,8 @@ impl Frame {
},
Bytecode::Pack(sd_idx) => {
let field_count = resolver.field_count(*sd_idx);
let struct_type = resolver.get_struct_type(*sd_idx);
check_depth_of_type(resolver, &struct_type)?;
gas_meter.charge_pack(
false,
interpreter.operand_stack.last_n(field_count as usize)?,
Expand All @@ -1881,6 +1968,8 @@ impl Frame {
},
Bytecode::PackGeneric(si_idx) => {
let field_count = resolver.field_instantiation_count(*si_idx);
let ty = resolver.instantiate_generic_type(*si_idx, self.ty_args())?;
check_depth_of_type(resolver, &ty)?;
gas_meter.charge_pack(
true,
interpreter.operand_stack.last_n(field_count as usize)?,
Expand Down Expand Up @@ -2197,6 +2286,7 @@ impl Frame {
},
Bytecode::VecPack(si, num) => {
let ty = resolver.instantiate_single_type(*si, self.ty_args())?;
check_depth_of_type(resolver, &ty)?;
gas_meter.charge_vec_pack(
make_ty!(&ty),
interpreter.operand_stack.last_n(*num as usize)?,
Expand Down
Loading

0 comments on commit 47a0391

Please sign in to comment.