From acdc5f4500c4f64d3fe4471a7beedf45a45d8f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Tue, 3 Oct 2023 13:39:11 +0000 Subject: [PATCH] Fix detection of runtime limits for accessors (#3335) * Fix detection of runtime limits for function calls * Fix tests * Add additional checks and extract function * cargo fmt * Fix build --- .../src/object/internal_methods/function.rs | 6 +++ boa_engine/src/vm/code_block.rs | 2 + boa_engine/src/vm/mod.rs | 22 ++++++-- boa_engine/src/vm/opcode/call/mod.rs | 54 ------------------- boa_engine/src/vm/opcode/new/mod.rs | 26 --------- boa_engine/src/vm/tests.rs | 4 +- 6 files changed, 29 insertions(+), 85 deletions(-) diff --git a/boa_engine/src/object/internal_methods/function.rs b/boa_engine/src/object/internal_methods/function.rs index 4515bf6074..92ceb2afde 100644 --- a/boa_engine/src/object/internal_methods/function.rs +++ b/boa_engine/src/object/internal_methods/function.rs @@ -89,6 +89,9 @@ pub(crate) fn native_function_call( args: &[JsValue], context: &mut Context<'_>, ) -> JsResult { + // We technically don't need this since native functions don't push any new frames to the + // vm, but we'll eventually have to combine the native stack with the vm stack. + context.check_runtime_limits()?; let this_function_object = obj.clone(); let object = obj.borrow(); @@ -135,6 +138,9 @@ fn native_function_construct( new_target: &JsObject, context: &mut Context<'_>, ) -> JsResult { + // We technically don't need this since native functions don't push any new frames to the + // vm, but we'll eventually have to combine the native stack with the vm stack. + context.check_runtime_limits()?; let this_function_object = obj.clone(); let object = obj.borrow(); diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 0528036387..a88d350673 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -1014,6 +1014,7 @@ impl JsObject { args: &[JsValue], context: &mut Context<'_>, ) -> JsResult { + context.check_runtime_limits()?; let old_realm = context.realm().clone(); let context = &mut context.guard(move |ctx| { @@ -1152,6 +1153,7 @@ impl JsObject { this_target: &JsValue, context: &mut Context<'_>, ) -> JsResult { + context.check_runtime_limits()?; let old_realm = context.realm().clone(); let context = &mut context.guard(move |ctx| { ctx.enter_realm(old_realm); diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index e443fe7d08..7267fd61b5 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -4,13 +4,11 @@ //! This module will provide an instruction set for the AST to use, various traits, //! plus an interpreter to execute those instructions -#[cfg(feature = "fuzz")] -use crate::JsNativeError; use crate::{ environments::{DeclarativeEnvironment, EnvironmentStack}, script::Script, vm::code_block::Readable, - Context, JsError, JsNativeErrorKind, JsObject, JsResult, JsValue, Module, + Context, JsError, JsNativeError, JsNativeErrorKind, JsObject, JsResult, JsValue, Module, }; use boa_gc::{custom_trace, Finalize, Gc, Trace}; @@ -412,4 +410,22 @@ impl Context<'_> { } } } + + /// Checks if we haven't exceeded the defined runtime limits. + pub(crate) fn check_runtime_limits(&self) -> JsResult<()> { + // Must throw if the number of recursive calls exceeds the defined limit. + if self.vm.runtime_limits.recursion_limit() <= self.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message("exceeded maximum number of recursive calls") + .into()); + } + // Must throw if the stack size exceeds the defined maximum length. + if self.vm.runtime_limits.stack_size_limit() <= self.vm.stack.len() { + return Err(JsNativeError::runtime_limit() + .with_message("exceeded maximum call stack length") + .into()); + } + + Ok(()) + } } diff --git a/boa_engine/src/vm/opcode/call/mod.rs b/boa_engine/src/vm/opcode/call/mod.rs index 2de9e45121..5d3547be19 100644 --- a/boa_engine/src/vm/opcode/call/mod.rs +++ b/boa_engine/src/vm/opcode/call/mod.rs @@ -16,19 +16,6 @@ pub(crate) struct CallEval; impl CallEval { fn operation(context: &mut Context<'_>, argument_count: usize) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } let mut arguments = Vec::with_capacity(argument_count); for _ in 0..argument_count { arguments.push(context.vm.pop()); @@ -95,20 +82,6 @@ impl Operation for CallEvalSpread { const INSTRUCTION: &'static str = "INST - CallEvalSpread"; fn execute(context: &mut Context<'_>) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } - // Get the arguments that are stored as an array object on the stack. let arguments_array = context.vm.pop(); let arguments_array_object = arguments_array @@ -158,19 +131,6 @@ pub(crate) struct Call; impl Call { fn operation(context: &mut Context<'_>, argument_count: usize) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } let mut arguments = Vec::with_capacity(argument_count); for _ in 0..argument_count { arguments.push(context.vm.pop()); @@ -221,20 +181,6 @@ impl Operation for CallSpread { const INSTRUCTION: &'static str = "INST - CallSpread"; fn execute(context: &mut Context<'_>) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } - // Get the arguments that are stored as an array object on the stack. let arguments_array = context.vm.pop(); let arguments_array_object = arguments_array diff --git a/boa_engine/src/vm/opcode/new/mod.rs b/boa_engine/src/vm/opcode/new/mod.rs index cf68063f35..00b3192831 100644 --- a/boa_engine/src/vm/opcode/new/mod.rs +++ b/boa_engine/src/vm/opcode/new/mod.rs @@ -13,19 +13,6 @@ pub(crate) struct New; impl New { fn operation(context: &mut Context<'_>, argument_count: usize) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } let mut arguments = Vec::with_capacity(argument_count); for _ in 0..argument_count { arguments.push(context.vm.pop()); @@ -79,19 +66,6 @@ impl Operation for NewSpread { const INSTRUCTION: &'static str = "INST - NewSpread"; fn execute(context: &mut Context<'_>) -> JsResult { - if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { - return Err(JsNativeError::runtime_limit() - .with_message(format!( - "Maximum recursion limit {} exceeded", - context.vm.runtime_limits.recursion_limit() - )) - .into()); - } - if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { - return Err(JsNativeError::runtime_limit() - .with_message("Maximum call stack size exceeded") - .into()); - } // Get the arguments that are stored as an array object on the stack. let arguments_array = context.vm.pop(); let arguments_array_object = arguments_array diff --git a/boa_engine/src/vm/tests.rs b/boa_engine/src/vm/tests.rs index 5cea152af7..4e473d23bd 100644 --- a/boa_engine/src/vm/tests.rs +++ b/boa_engine/src/vm/tests.rs @@ -293,7 +293,7 @@ fn recursion_runtime_limit() { TestAction::assert_native_error( "factorial(11)", JsNativeErrorKind::RuntimeLimit, - "Maximum recursion limit 10 exceeded", + "exceeded maximum number of recursive calls", ), TestAction::assert_eq("factorial(8)", JsValue::new(40_320)), TestAction::assert_native_error( @@ -305,7 +305,7 @@ fn recursion_runtime_limit() { x() "#}, JsNativeErrorKind::RuntimeLimit, - "Maximum recursion limit 10 exceeded", + "exceeded maximum number of recursive calls", ), ]); }