From be97047e512c212bad513557d249bd967d5d26fa Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 15 May 2020 21:40:54 +0800 Subject: [PATCH] Limit modules loading. --- README.md | 20 ++++++++-- src/any.rs | 8 ++-- src/api.rs | 6 +-- src/engine.rs | 95 ++++++++++++++++++++++++++------------------- src/fn_func.rs | 2 +- src/fn_native.rs | 4 +- src/fn_register.rs | 6 +-- src/lib.rs | 1 + src/module.rs | 8 ++-- src/optimize.rs | 1 - src/packages/mod.rs | 4 +- src/result.rs | 6 +++ src/scope.rs | 2 +- src/unsafe.rs | 2 - tests/modules.rs | 51 +++++++++++++++++++++++- 15 files changed, 147 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index c602fa92..7edc5667 100644 --- a/README.md +++ b/README.md @@ -23,8 +23,8 @@ Rhai's current features set: * Relatively little `unsafe` code (yes there are some for performance reasons) * Sand-boxed (the scripting [`Engine`] can be declared immutable which cannot mutate the containing environment unless explicitly allowed via `RefCell` etc.) -* Rugged (protection against [stack-overflow](#maximum-stack-depth) and [runaway scripts](#maximum-number-of-operations)) -* Able to set limits on script resource usage (e.g. see [tracking progress](#tracking-progress)) +* Rugged (protection against [stack-overflow](#maximum-stack-depth) and [runaway scripts](#maximum-number-of-operations) etc.) +* Able to track script evaluation [progress](#tracking-progress) and manually terminate a script run * [`no-std`](#optional-features) support * [Function overloading](#function-overloading) * [Operator overloading](#operator-overloading) @@ -2268,6 +2268,7 @@ The most important resources to watch out for are: floating-point representations, in order to crash the system. * **Files**: A malignant script may continuously `import` an external module within an infinite loop, thereby putting heavy load on the file-system (or even the network if the file is not local). + Even when modules are not created from files, they still typically consume a lot of resources to load. * **Data**: A malignant script may attempt to read from and/or write to data that it does not own. If this happens, it is a severe security breach and may put the entire system at risk. @@ -2319,6 +2320,19 @@ engine.on_progress(|count| { // 'count' is the number of operatio The closure passed to `Engine::on_progress` will be called once every operation. Return `false` to terminate the script immediately. +### Maximum number of modules + +Rhai by default does not limit how many [modules] are loaded via the `import` statement. +This can be changed via the `Engine::set_max_modules` method, with zero being unlimited (the default). + +```rust +let mut engine = Engine::new(); + +engine.set_max_modules(5); // allow loading only up to 5 modules + +engine.set_max_modules(0); // allow unlimited modules +``` + ### Maximum stack depth Rhai by default limits function calls to a maximum depth of 256 levels (28 levels in debug build). @@ -2646,7 +2660,7 @@ let x = eval("40 + 2"); // 'eval' here throws "eval is evil! I refuse to run Or override it from Rust: ```rust -fn alt_eval(script: String) -> Result<(), EvalAltResult> { +fn alt_eval(script: String) -> Result<(), Box> { Err(format!("eval is evil! I refuse to run {}", script).into()) } diff --git a/src/any.rs b/src/any.rs index 483eee84..7f209685 100644 --- a/src/any.rs +++ b/src/any.rs @@ -19,7 +19,7 @@ use crate::stdlib::{ any::{type_name, Any, TypeId}, boxed::Box, collections::HashMap, - fmt, mem, ptr, + fmt, string::String, vec::Vec, }; @@ -27,7 +27,7 @@ use crate::stdlib::{ #[cfg(not(feature = "no_std"))] use crate::stdlib::time::Instant; -/// A trait to represent any type. +/// Trait to represent any type. /// /// Currently, `Variant` is not `Send` nor `Sync`, so it can practically be any type. /// Turn on the `sync` feature to restrict it to only types that implement `Send + Sync`. @@ -81,7 +81,7 @@ impl Variant for T { } } -/// A trait to represent any type. +/// Trait to represent any type. /// /// `From<_>` is implemented for `i64` (`i32` if `only_i32`), `f64` (if not `no_float`), /// `bool`, `String`, `char`, `Vec` (into `Array`) and `HashMap` (into `Map`). @@ -142,7 +142,7 @@ impl dyn Variant { } } -/// A dynamic type containing any value. +/// Dynamic type containing any value. pub struct Dynamic(pub(crate) Union); /// Internal `Dynamic` representation. diff --git a/src/api.rs b/src/api.rs index 78ceaf72..5e144bcf 100644 --- a/src/api.rs +++ b/src/api.rs @@ -21,7 +21,6 @@ use crate::engine::Map; use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, - collections::HashMap, mem, string::{String, ToString}, }; @@ -1016,11 +1015,10 @@ impl Engine { .get_function_by_signature(name, args.len(), true) .ok_or_else(|| Box::new(EvalAltResult::ErrorFunctionNotFound(name.into(), pos)))?; - let mut state = State::new(fn_lib); + let state = State::new(fn_lib); let args = args.as_mut(); - let (result, _) = - self.call_script_fn(Some(scope), &mut state, name, fn_def, args, pos, 0, 0)?; + let (result, _) = self.call_script_fn(Some(scope), state, name, fn_def, args, pos, 0)?; let return_type = self.map_type_name(result.type_name()); diff --git a/src/engine.rs b/src/engine.rs index 6f159487..6423ca6b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -22,7 +22,6 @@ use crate::parser::ModuleRef; use crate::stdlib::{ any::TypeId, - borrow::Cow, boxed::Box, collections::HashMap, format, @@ -36,13 +35,13 @@ use crate::stdlib::{ vec::Vec, }; -/// An dynamic array of `Dynamic` values. +/// Variable-sized array of `Dynamic` values. /// /// Not available under the `no_index` feature. #[cfg(not(feature = "no_index"))] pub type Array = Vec; -/// An dynamic hash map of `Dynamic` values with `String` keys. +/// Hash map of `Dynamic` values with `String` keys. /// /// Not available under the `no_object` feature. #[cfg(not(feature = "no_object"))] @@ -154,16 +153,20 @@ pub struct State<'a> { /// Number of operations performed. pub operations: u64, + + /// Number of modules loaded. + pub modules: u64, } impl<'a> State<'a> { /// Create a new `State`. pub fn new(fn_lib: &'a FunctionsLib) -> Self { Self { - always_search: false, fn_lib, + always_search: false, scope_level: 0, operations: 0, + modules: 0, } } /// Does a certain script-defined function exist in the `State`? @@ -322,8 +325,10 @@ pub struct Engine { /// /// Defaults to 28 for debug builds and 256 for non-debug builds. pub(crate) max_call_stack_depth: usize, - /// Maximum number of operations to run. + /// Maximum number of operations allowed to run. pub(crate) max_operations: Option, + /// Maximum number of modules allowed to load. + pub(crate) max_modules: Option, } impl Default for Engine { @@ -363,6 +368,7 @@ impl Default for Engine { max_call_stack_depth: MAX_CALL_STACK_DEPTH, max_operations: None, + max_modules: None, }; #[cfg(feature = "no_stdlib")] @@ -503,6 +509,7 @@ impl Engine { max_call_stack_depth: MAX_CALL_STACK_DEPTH, max_operations: None, + max_modules: None, } } @@ -545,6 +552,13 @@ impl Engine { pub fn set_max_operations(&mut self, operations: u64) { self.max_operations = NonZeroU64::new(operations); } + + /// Set the maximum number of imported modules allowed for a script (0 for unlimited). + #[cfg(not(feature = "unchecked"))] + pub fn set_max_modules(&mut self, modules: u64) { + self.max_modules = NonZeroU64::new(modules); + } + /// Set the module resolution service used by the `Engine`. /// /// Not available under the `no_module` feature. @@ -582,11 +596,9 @@ impl Engine { // First search in script-defined functions (can override built-in) if hashes.1 > 0 { if let Some(fn_def) = state.get_function(hashes.1) { - let ops = state.operations; - let (result, operations) = - self.call_script_fn(scope, &state, fn_name, fn_def, args, pos, level, ops)?; - state.operations = operations; - self.inc_operations(state, pos)?; + let (result, state2) = + self.call_script_fn(scope, *state, fn_name, fn_def, args, pos, level)?; + *state = state2; return Ok((result, false)); } } @@ -701,24 +713,23 @@ impl Engine { /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn call_script_fn<'s>( &self, - scope: Option<&mut Scope<'s>>, - state: &State, + scope: Option<&mut Scope>, + mut state: State<'s>, fn_name: &str, fn_def: &FnDef, args: &mut FnCallArgs, pos: Position, level: usize, - operations: u64, - ) -> Result<(Dynamic, u64), Box> { + ) -> Result<(Dynamic, State<'s>), Box> { + self.inc_operations(&mut state, pos)?; + + let orig_scope_level = state.scope_level; + state.scope_level += 1; + match scope { // Extern scope passed in which is not empty Some(scope) if scope.len() > 0 => { let scope_len = scope.len(); - let mut local_state = State::new(state.fn_lib); - - local_state.operations = operations; - self.inc_operations(&mut local_state, pos)?; - local_state.scope_level += 1; // Put arguments into scope as variables scope.extend( @@ -730,14 +741,14 @@ impl Engine { args.into_iter().map(|v| mem::take(*v)), ) .map(|(name, value)| { - let var_name = unsafe_cast_var_name(name.as_str(), &local_state); + let var_name = unsafe_cast_var_name(name.as_str(), &mut state); (var_name, ScopeEntryType::Normal, value) }), ); // Evaluate the function at one higher level of call depth let result = self - .eval_stmt(scope, &mut local_state, &fn_def.body, level + 1) + .eval_stmt(scope, &mut state, &fn_def.body, level + 1) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -756,19 +767,14 @@ impl Engine { }); // Remove all local variables - // No need to reset `state.scope_level` because it is thrown away scope.rewind(scope_len); + state.scope_level = orig_scope_level; - return result.map(|v| (v, local_state.operations)); + return result.map(|v| (v, state)); } // No new scope - create internal scope _ => { let mut scope = Scope::new(); - let mut local_state = State::new(state.fn_lib); - - local_state.operations = operations; - self.inc_operations(&mut local_state, pos)?; - local_state.scope_level += 1; // Put arguments into scope as variables scope.extend( @@ -783,9 +789,8 @@ impl Engine { ); // Evaluate the function at one higher level of call depth - // No need to reset `state.scope_level` because it is thrown away - return self - .eval_stmt(&mut scope, &mut local_state, &fn_def.body, level + 1) + let result = self + .eval_stmt(&mut scope, &mut state, &fn_def.body, level + 1) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -801,8 +806,10 @@ impl Engine { err, pos, ))), - }) - .map(|v| (v, local_state.operations)); + }); + + state.scope_level = orig_scope_level; + return result.map(|v| (v, state)); } } } @@ -1512,11 +1519,9 @@ impl Engine { // First search in script-defined functions (can override built-in) if let Some(fn_def) = module.get_qualified_scripted_fn(*hash_fn_def) { let args = args.as_mut(); - let ops = state.operations; - let (result, operations) = - self.call_script_fn(None, state, name, fn_def, args, *pos, level, ops)?; - state.operations = operations; - self.inc_operations(state, *pos)?; + let (result, state2) = + self.call_script_fn(None, *state, name, fn_def, args, *pos, level)?; + *state = state2; Ok(result) } else { // Then search in Rust functions @@ -1792,13 +1797,20 @@ impl Engine { // Import statement Stmt::Import(x) => { - let (expr, (name, pos)) = x.as_ref(); - #[cfg(feature = "no_module")] unreachable!(); #[cfg(not(feature = "no_module"))] { + let (expr, (name, pos)) = x.as_ref(); + + // Guard against too many modules + if let Some(max) = self.max_modules { + if state.modules >= max.get() { + return Err(Box::new(EvalAltResult::ErrorTooManyModules(*pos))); + } + } + if let Some(path) = self .eval_expr(scope, state, &expr, level)? .try_cast::() @@ -1810,7 +1822,10 @@ impl Engine { let mod_name = unsafe_cast_var_name(name, &state); scope.push_module(mod_name, module); + + state.modules += 1; self.inc_operations(state, *pos)?; + Ok(Default::default()) } else { Err(Box::new(EvalAltResult::ErrorModuleNotFound( diff --git a/src/fn_func.rs b/src/fn_func.rs index e619ebdf..f606b794 100644 --- a/src/fn_func.rs +++ b/src/fn_func.rs @@ -11,7 +11,7 @@ use crate::scope::Scope; use crate::stdlib::{boxed::Box, string::ToString}; -/// A trait to create a Rust anonymous function from a script. +/// Trait to create a Rust anonymous function from a script. pub trait Func { type Output; diff --git a/src/fn_native.rs b/src/fn_native.rs index 335f9919..7a1dd0e6 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -82,7 +82,7 @@ pub enum NativeFunctionABI { Method, } -/// A trait implemented by all native Rust functions that are callable by Rhai. +/// Trait implemented by all native Rust functions that are callable by Rhai. #[cfg(not(feature = "sync"))] pub trait NativeCallable { /// Get the ABI type of a native Rust function. @@ -91,7 +91,7 @@ pub trait NativeCallable { fn call(&self, args: &mut FnCallArgs) -> Result>; } -/// A trait implemented by all native Rust functions that are callable by Rhai. +/// Trait implemented by all native Rust functions that are callable by Rhai. #[cfg(feature = "sync")] pub trait NativeCallable: Send + Sync { /// Get the ABI type of a native Rust function. diff --git a/src/fn_register.rs b/src/fn_register.rs index 8b91ea42..42e81c9b 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -10,7 +10,7 @@ use crate::result::EvalAltResult; use crate::stdlib::{any::TypeId, boxed::Box, mem, string::ToString}; -/// A trait to register custom functions with the `Engine`. +/// Trait to register custom functions with the `Engine`. pub trait RegisterFn { /// Register a custom function with the `Engine`. /// @@ -42,7 +42,7 @@ pub trait RegisterFn { fn register_fn(&mut self, name: &str, f: FN); } -/// A trait to register custom functions that return `Dynamic` values with the `Engine`. +/// Trait to register custom functions that return `Dynamic` values with the `Engine`. pub trait RegisterDynamicFn { /// Register a custom function returning `Dynamic` values with the `Engine`. /// @@ -69,7 +69,7 @@ pub trait RegisterDynamicFn { fn register_dynamic_fn(&mut self, name: &str, f: FN); } -/// A trait to register fallible custom functions returning `Result<_, Box>` with the `Engine`. +/// Trait to register fallible custom functions returning `Result<_, Box>` with the `Engine`. pub trait RegisterResultFn { /// Register a custom fallible function with the `Engine`. /// diff --git a/src/lib.rs b/src/lib.rs index f5f1ad87..203a6dfb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,6 +116,7 @@ pub use parser::FLOAT; #[cfg(not(feature = "no_module"))] pub use module::ModuleResolver; +/// Module containing all built-in _module resolvers_ available to Rhai. #[cfg(not(feature = "no_module"))] pub mod module_resolvers { pub use crate::module::resolvers::*; diff --git a/src/module.rs b/src/module.rs index 6e56a128..f65428a0 100644 --- a/src/module.rs +++ b/src/module.rs @@ -769,7 +769,7 @@ impl ModuleRef { } } -/// A trait that encapsulates a module resolution service. +/// Trait that encapsulates a module resolution service. #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "sync"))] pub trait ModuleResolver { @@ -783,7 +783,7 @@ pub trait ModuleResolver { ) -> Result>; } -/// A trait that encapsulates a module resolution service. +/// Trait that encapsulates a module resolution service. #[cfg(not(feature = "no_module"))] #[cfg(feature = "sync")] pub trait ModuleResolver: Send + Sync { @@ -812,7 +812,7 @@ mod file { use super::*; use crate::stdlib::path::PathBuf; - /// A module resolution service that loads module script files from the file system. + /// Module resolution service that loads module script files from the file system. /// /// The `new_with_path` and `new_with_path_and_extension` constructor functions /// allow specification of a base directory with module path used as a relative path offset @@ -949,7 +949,7 @@ mod file { mod stat { use super::*; - /// A module resolution service that serves modules added into it. + /// Module resolution service that serves modules added into it. /// /// # Examples /// diff --git a/src/optimize.rs b/src/optimize.rs index df385b74..04026059 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -13,7 +13,6 @@ use crate::token::Position; use crate::stdlib::{ boxed::Box, - collections::HashMap, iter::empty, string::{String, ToString}, vec, diff --git a/src/packages/mod.rs b/src/packages/mod.rs index 5c2a6dd5..7c146f7f 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -1,4 +1,4 @@ -//! This module contains all built-in _packages_ available to Rhai, plus facilities to define custom packages. +//! Module containing all built-in _packages_ available to Rhai, plus facilities to define custom packages. use crate::fn_native::{NativeCallable, SharedIteratorFunction}; use crate::module::Module; @@ -89,7 +89,7 @@ impl PackagesCollection { } } -/// This macro makes it easy to define a _package_ (which is basically a shared module) +/// Macro that makes it easy to define a _package_ (which is basically a shared module) /// and register functions into it. /// /// Functions can be added to the package using the standard module methods such as diff --git a/src/result.rs b/src/result.rs index 455a67ce..4ae0d9e5 100644 --- a/src/result.rs +++ b/src/result.rs @@ -81,6 +81,8 @@ pub enum EvalAltResult { ErrorArithmetic(String, Position), /// Number of operations over maximum limit. ErrorTooManyOperations(Position), + /// Modules over maximum limit. + ErrorTooManyModules(Position), /// Call stack over maximum limit. ErrorStackOverflow(Position), /// The script is prematurely terminated. @@ -142,6 +144,7 @@ impl EvalAltResult { Self::ErrorDotExpr(_, _) => "Malformed dot expression", Self::ErrorArithmetic(_, _) => "Arithmetic error", Self::ErrorTooManyOperations(_) => "Too many operations", + Self::ErrorTooManyModules(_) => "Too many modules imported", Self::ErrorStackOverflow(_) => "Stack overflow", Self::ErrorTerminated(_) => "Script terminated.", Self::ErrorRuntime(_, _) => "Runtime error", @@ -190,6 +193,7 @@ impl fmt::Display for EvalAltResult { | Self::ErrorInExpr(pos) | Self::ErrorDotExpr(_, pos) | Self::ErrorTooManyOperations(pos) + | Self::ErrorTooManyModules(pos) | Self::ErrorStackOverflow(pos) | Self::ErrorTerminated(pos) => write!(f, "{} ({})", desc, pos), @@ -308,6 +312,7 @@ impl EvalAltResult { | Self::ErrorDotExpr(_, pos) | Self::ErrorArithmetic(_, pos) | Self::ErrorTooManyOperations(pos) + | Self::ErrorTooManyModules(pos) | Self::ErrorStackOverflow(pos) | Self::ErrorTerminated(pos) | Self::ErrorRuntime(_, pos) @@ -346,6 +351,7 @@ impl EvalAltResult { | Self::ErrorDotExpr(_, pos) | Self::ErrorArithmetic(_, pos) | Self::ErrorTooManyOperations(pos) + | Self::ErrorTooManyModules(pos) | Self::ErrorStackOverflow(pos) | Self::ErrorTerminated(pos) | Self::ErrorRuntime(_, pos) diff --git a/src/scope.rs b/src/scope.rs index 17909970..1b9c8b1d 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -36,7 +36,7 @@ pub struct Entry<'a> { pub expr: Option>, } -/// A type containing information about the current scope. +/// Type containing information about the current scope. /// Useful for keeping state between `Engine` evaluation runs. /// /// # Example diff --git a/src/unsafe.rs b/src/unsafe.rs index 47d5b40c..65ae0b45 100644 --- a/src/unsafe.rs +++ b/src/unsafe.rs @@ -2,7 +2,6 @@ use crate::any::Variant; use crate::engine::State; -use crate::utils::StaticVec; use crate::stdlib::{ any::{Any, TypeId}, @@ -10,7 +9,6 @@ use crate::stdlib::{ boxed::Box, mem, ptr, string::ToString, - vec::Vec, }; /// Cast a type into another type. diff --git a/tests/modules.rs b/tests/modules.rs index acf3c0e6..569cb4af 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -72,13 +72,60 @@ fn test_module_resolver() -> Result<(), Box> { assert_eq!( engine.eval::( r#" - import "hello" as h; - h::answer + import "hello" as h1; + import "hello" as h2; + h2::answer "# )?, 42 ); + engine.set_max_modules(5); + + assert!(matches!( + *engine + .eval::<()>( + r#" + for x in range(0, 10) { + import "hello" as h; + } + "# + ) + .expect_err("should error"), + EvalAltResult::ErrorTooManyModules(_) + )); + + assert!(matches!( + *engine + .eval::<()>( + r#" + fn foo() { + import "hello" as h; + } + + for x in range(0, 10) { + foo(); + } + "# + ) + .expect_err("should error"), + EvalAltResult::ErrorInFunctionCall(fn_name, _, _) if fn_name == "foo" + )); + + engine.set_max_modules(0); + + engine.eval::<()>( + r#" + fn foo() { + import "hello" as h; + } + + for x in range(0, 10) { + foo(); + } + "#, + )?; + Ok(()) }