diff --git a/CHANGELOG.md b/CHANGELOG.md index 4127dde4..994b36c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Bug fixes * `chop` for arrays and BLOB's now works properly. * `set_bit` for bit-flags with negative index now works correctly. * Misnamed `params` field `name` in the JSON output of `Engine::gen_fn_metadata_to_json` is fixed (was incorrectly named `type`). +* Fixes a potential `unsafe` violation in `for` loop. Enhancements ------------ diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index cb2fa390..aff46fe6 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -464,7 +464,7 @@ impl Engine { // For loop Stmt::For(expr, x, _) => { - let (Ident { name, .. }, counter, statements) = x.as_ref(); + let (Ident { name: var_name, .. }, counter, statements) = x.as_ref(); let iter_obj = self .eval_expr(scope, global, state, lib, this_ptr, expr, level)? .flatten(); @@ -492,14 +492,23 @@ impl Engine { // Add the loop variables let orig_scope_len = scope.len(); let counter_index = if let Some(counter) = counter { - scope.push(unsafe_cast_var_name_to_lifetime(&counter.name), 0 as INT); + // Loop variables are always removed at the end of the statement + // so this cast is safe. + let counter_name = unsafe_cast_var_name_to_lifetime(&counter.name); + scope.push(counter_name, 0 as INT); scope.len() - 1 } else { usize::MAX }; - scope.push(unsafe_cast_var_name_to_lifetime(name), ()); + + // Loop variables are always removed at the end of the statement + // so this cast is safe. + let var_name = unsafe_cast_var_name_to_lifetime(var_name); + scope.push(var_name, ()); let index = scope.len() - 1; + let mut loop_result = Ok(Dynamic::UNIT); + for (x, iter_value) in func(iter_obj).enumerate() { // Increment counter if counter_index < usize::MAX { @@ -546,7 +555,12 @@ impl Engine { } #[cfg(not(feature = "unchecked"))] - self.inc_operations(&mut global.num_operations, statements.position())?; + if let Err(err) = + self.inc_operations(&mut global.num_operations, statements.position()) + { + loop_result = Err(err); + break; + } if statements.is_empty() { continue; @@ -567,7 +581,8 @@ impl Engine { } scope.rewind(orig_scope_len); - Ok(Dynamic::UNIT) + + loop_result } else { Err(ERR::ErrorFor(expr.position()).into()) } @@ -580,7 +595,7 @@ impl Engine { // Try/Catch statement Stmt::TryCatch(x, _) => { - let (try_stmt, err_var, catch_stmt) = x.as_ref(); + let (try_stmt, err_var_name, catch_stmt) = x.as_ref(); let result = self .eval_stmt_block(scope, global, state, lib, this_ptr, try_stmt, true, level) @@ -630,8 +645,11 @@ impl Engine { let orig_scope_len = scope.len(); - err_var.as_ref().map(|Ident { name, .. }| { - scope.push(unsafe_cast_var_name_to_lifetime(name), err_value) + err_var_name.as_ref().map(|Ident { name, .. }| { + // Catch error variables are always removed from after the block + // so this cast is safe. + let var_name = unsafe_cast_var_name_to_lifetime(name); + scope.push(var_name, err_value) }); let result = self.eval_stmt_block( diff --git a/src/func/script.rs b/src/func/script.rs index cc37333a..51d7305e 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -74,14 +74,18 @@ impl Engine { let orig_mods_len = global.num_imported_modules(); // Put arguments into scope as variables - // Actually consume the arguments instead of cloning them scope.extend( fn_def .params .iter() - .zip(args.iter_mut().map(|v| mem::take(*v))) + .zip(args.into_iter().map(|v| { + // Actually consume the arguments instead of cloning them + mem::take(*v) + })) .map(|(name, value)| { - let var_name: std::borrow::Cow<'_, str> = + // Arguments are always removed at the end of the call, + // so this cast is safe. + let var_name: std::borrow::Cow<_> = unsafe_cast_var_name_to_lifetime(name).into(); (var_name, value) }),