Fix range overflow panics.

CHANGELOG.md

@@ -10,6 +10,7 @@ Bug fixes
 * Errors in native Rust functions now contain the correct function call positions.
 * Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped.
 * Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive.
+* Potential overflow panics in `range(from, to, step)` is fixed.
 
 Breaking changes
 ----------------
@@ -23,6 +24,7 @@ Breaking changes
 * Function keywords (e.g. `type_of`, `eval`, `Fn`) can no longer be overloaded. It is more trouble than worth. To disable these keywords, use `Engine::disable_symbol`.
 * `is_def_var` and `is_def_fn` are now reserved keywords.
 * `Engine::id` field is removed.
+* `num-traits` is now a required dependency.
 
 Enhancements
 ------------

Cargo.toml

@@ -26,6 +26,7 @@ categories = [ "no-std", "embedded", "wasm", "parser-implementations" ]
 [dependencies]
 smallvec = { version = "1.6", default-features = false, features = ["union"] }
 ahash = { version = "0.6", default-features = false }
+num-traits = { version = "0.2", default_features = false }
 rhai_codegen = { version = "0.3.3", path = "codegen" }
 
 [features]
@@ -65,11 +66,6 @@ version = "0.2"
 default_features = false
 optional = true
 
-[dependencies.num-traits]
-version = "0.2"
-default-features = false
-optional = true
-
 [dependencies.core-error]
 version = "0.0"
 default_features = false

README.md

@@ -35,7 +35,7 @@ Standard features
 * Built-in support for most common [data types](https://rhai.rs/book/language/values-and-types.html) including booleans, integers, floating-point numbers (including [`Decimal`](https://crates.io/crates/rust_decimal)), strings, Unicode characters, arrays and maps.
 * Easily [call a script-defined function](https://rhai.rs/book/engine/call-fn.html) from Rust.
 * Relatively little `unsafe` code (yes there are some for performance reasons).
-* Few dependencies (currently only [`smallvec`](https://crates.io/crates/smallvec) and [`ahash`](https://crates.io/crates/ahash)).
+* Few dependencies (currently only [`smallvec`](https://crates.io/crates/smallvec), [`num-traits`](https://crates.io/crates/num-traits) and [`ahash`](https://crates.io/crates/ahash)).
 * Re-entrant scripting engine can be made `Send + Sync` (via the `sync` feature).
 * Compile once to [AST](https://rhai.rs/book/engine/compile.html) form for repeated evaluations.
 * Scripts are [optimized](https://rhai.rs/book/engine/optimize.html) (useful for template-based machine-generated scripts).

src/packages/iter_basic.rs

@@ -1,10 +1,18 @@
 use crate::dynamic::Variant;
-use crate::stdlib::{
+use crate::stdlib::{boxed::Box, ops::Range};
-    boxed::Box,
+use crate::{def_package, EvalAltResult, INT};
-    ops::{Add, Range, Sub},
+
-    string::ToString,
+#[cfg(not(feature = "unchecked"))]
-};
+use crate::stdlib::string::ToString;
-use crate::{def_package, EvalAltResult, Position, INT};
+
+#[cfg(feature = "decimal")]
+use rust_decimal::Decimal;
+
+#[cfg(not(feature = "unchecked"))]
+use num_traits::{CheckedAdd as Add, CheckedSub as Sub};
+
+#[cfg(feature = "unchecked")]
+use crate::stdlib::ops::{Add, Sub};
 
 fn get_range<T: Variant + Clone>(from: T, to: T) -> Result<Range<T>, Box<EvalAltResult>> {
     Ok(from..to)
@@ -14,30 +22,35 @@ fn get_range<T: Variant + Clone>(from: T, to: T) -> Result<Range<T>, Box<EvalAlt
 #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)]
 struct StepRange<T>(T, T, T)
 where
-    for<'a> &'a T: Add<&'a T, Output = T>,
+    T: Variant + Copy + PartialOrd + Add<Output = T> + Sub<Output = T>;
-    T: Variant + Clone + PartialOrd;
 
 impl<T> StepRange<T>
 where
-    for<'a> &'a T: Add<&'a T, Output = T> + Sub<&'a T, Output = T>,
+    T: Variant + Copy + PartialOrd + Add<Output = T> + Sub<Output = T>,
-    T: Variant + Clone + PartialOrd,
 {
     pub fn new(from: T, to: T, step: T) -> Result<Self, Box<EvalAltResult>> {
-        if &from + &step == from {
+        #[cfg(not(feature = "unchecked"))]
-            Err(Box::new(EvalAltResult::ErrorArithmetic(
+        if let Some(r) = from.checked_add(&step) {
-                "step value cannot be zero".to_string(),
+            if r == from {
-                Position::NONE,
+                return Err(Box::new(EvalAltResult::ErrorInFunctionCall(
-            )))
+                    "range".to_string(),
-        } else {
+                    "".to_string(),
-            Ok(Self(from, to, step))
+                    Box::new(EvalAltResult::ErrorArithmetic(
+                        "step value cannot be zero".to_string(),
+                        crate::Position::NONE,
+                    )),
+                    crate::Position::NONE,
+                )));
+            }
         }
+
+        Ok(Self(from, to, step))
     }
 }
 
 impl<T> Iterator for StepRange<T>
 where
-    for<'a> &'a T: Add<&'a T, Output = T> + Sub<&'a T, Output = T>,
+    T: Variant + Copy + PartialOrd + Add<Output = T> + Sub<Output = T>,
-    T: Variant + Clone + PartialOrd,
 {
     type Item = T;
 
@@ -45,31 +58,75 @@ where
         if self.0 == self.1 {
             None
         } else if self.0 < self.1 {
-            let diff1 = &self.1 - &self.0;
+            #[cfg(not(feature = "unchecked"))]
+            let diff1 = if let Some(diff) = self.1.checked_sub(&self.0) {
+                diff
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let diff1 = self.1 - self.0;
 
-            let v = self.0.clone();
+            let v = self.0;
-            let n = self.0.add(&self.2);
 
-            let diff2 = &self.1 - &n;
+            #[cfg(not(feature = "unchecked"))]
+            let n = if let Some(num) = self.0.checked_add(&self.2) {
+                num
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let n = self.0 + self.2;
+
+            #[cfg(not(feature = "unchecked"))]
+            let diff2 = if let Some(diff) = self.1.checked_sub(&n) {
+                diff
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let diff2 = self.1 - n;
 
             if diff2 >= diff1 {
                 None
             } else {
-                self.0 = if n >= self.1 { self.1.clone() } else { n };
+                self.0 = if n >= self.1 { self.1 } else { n };
                 Some(v)
             }
         } else {
-            let diff1 = &self.0 - &self.1;
+            #[cfg(not(feature = "unchecked"))]
+            let diff1 = if let Some(diff) = self.0.checked_sub(&self.1) {
+                diff
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let diff1 = self.0 - self.1;
 
-            let v = self.0.clone();
+            let v = self.0;
-            let n = self.0.add(&self.2);
 
-            let diff2 = &n - &self.1;
+            #[cfg(not(feature = "unchecked"))]
+            let n = if let Some(num) = self.0.checked_add(&self.2) {
+                num
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let n = self.0 + self.2;
+
+            #[cfg(not(feature = "unchecked"))]
+            let diff2 = if let Some(diff) = n.checked_sub(&self.1) {
+                diff
+            } else {
+                return None;
+            };
+            #[cfg(feature = "unchecked")]
+            let diff2 = n - self.1;
 
             if diff2 >= diff1 {
                 None
             } else {
-                self.0 = if n <= self.1 { self.1.clone() } else { n };
+                self.0 = if n <= self.1 { self.1 } else { n };
                 Some(v)
             }
         }
@@ -78,14 +135,13 @@ where
 
 fn get_step_range<T>(from: T, to: T, step: T) -> Result<StepRange<T>, Box<EvalAltResult>>
 where
-    for<'a> &'a T: Add<&'a T, Output = T> + Sub<&'a T, Output = T>,
+    T: Variant + Copy + PartialOrd + Add<Output = T> + Sub<Output = T>,
-    T: Variant + Clone + PartialOrd,
 {
     StepRange::<T>::new(from, to, step)
 }
 
 macro_rules! reg_range {
-    ($lib:expr, $x:expr, $( $y:ty ),*) => (
+    ($lib:ident | $x:expr => $( $y:ty ),*) => {
         $(
             $lib.set_iterator::<Range<$y>>();
             let hash = $lib.set_fn_2($x, get_range::<$y>);
@@ -95,11 +151,8 @@ macro_rules! reg_range {
                     concat!("Iterator<Item=", stringify!($y), ">")
             ]);
         )*
-    )
+    };
-}
+    ($lib:ident | step $x:expr => $( $y:ty ),*) => {
-
-macro_rules! reg_stepped_range {
-    ($lib:expr, $x:expr, $( $y:ty ),*) => (
         $(
             $lib.set_iterator::<StepRange<$y>>();
             let hash = $lib.set_fn_3($x, get_step_range::<$y>);
@@ -110,31 +163,93 @@ macro_rules! reg_stepped_range {
                     concat!("Iterator<Item=", stringify!($y), ">")
             ]);
         )*
-    )
+    };
 }
 
 def_package!(crate:BasicIteratorPackage:"Basic range iterators.", lib, {
-    reg_range!(lib, "range", INT);
+    reg_range!(lib | "range" => INT);
 
     #[cfg(not(feature = "only_i32"))]
     #[cfg(not(feature = "only_i64"))]
     {
-        reg_range!(lib, "range", i8, u8, i16, u16, i32, u32, i64, u64);
+        reg_range!(lib | "range" => i8, u8, i16, u16, i32, u32, i64, u64);
 
         if cfg!(not(target_arch = "wasm32")) {
-            reg_range!(lib, "range", i128, u128);
+            reg_range!(lib | "range" => i128, u128);
         }
     }
 
-    reg_stepped_range!(lib, "range", INT);
+    reg_range!(lib | step "range" => INT);
 
     #[cfg(not(feature = "only_i32"))]
     #[cfg(not(feature = "only_i64"))]
     {
-        reg_stepped_range!(lib, "range", i8, u8, i16, u16, i32, u32, i64, u64);
+        reg_range!(lib | step "range" => i8, u8, i16, u16, i32, u32, i64, u64);
 
         if cfg!(not(target_arch = "wasm32")) {
-            reg_stepped_range!(lib, "range", i128, u128);
+            reg_range!(lib | step "range" => i128, u128);
         }
     }
+
+    #[cfg(feature = "decimal")]
+    {
+        #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)]
+        struct StepDecimalRange(Decimal, Decimal, Decimal);
+
+        impl StepDecimalRange {
+            pub fn new(from: Decimal, to: Decimal, step: Decimal) -> Result<Self, Box<EvalAltResult>> {
+                #[cfg(not(feature = "unchecked"))]
+                if step.is_zero() {
+                    use crate::stdlib::string::ToString;
+
+                    return Err(Box::new(EvalAltResult::ErrorInFunctionCall("range".to_string(), "".to_string(),
+                        Box::new(EvalAltResult::ErrorArithmetic("step value cannot be zero".to_string(), crate::Position::NONE)),
+                        crate::Position::NONE,
+                    )));
+                }
+
+                Ok(Self(from, to, step))
+            }
+        }
+
+        impl Iterator for StepDecimalRange {
+            type Item = Decimal;
+
+            fn next(&mut self) -> Option<Decimal> {
+                if self.0 == self.1 {
+                    None
+                } else if self.0 < self.1 {
+                    #[cfg(not(feature = "unchecked"))]
+                    if self.2.is_sign_negative() {
+                        return None;
+                    }
+
+                    let v = self.0;
+                    let n = self.0 + self.2;
+
+                    self.0 = if n >= self.1 { self.1 } else { n };
+                    Some(v)
+                } else {
+                    #[cfg(not(feature = "unchecked"))]
+                    if self.2.is_sign_positive() {
+                        return None;
+                    }
+
+                    let v = self.0;
+                    let n = self.0 + self.2;
+
+                    self.0 = if n <= self.1 { self.1 } else { n };
+                    Some(v)
+                }
+            }
+        }
+
+        lib.set_iterator::<StepDecimalRange>();
+
+        let hash = lib.set_fn_2("range", |from, to| StepDecimalRange::new(from, to, Decimal::one()));
+        lib.update_fn_metadata(hash, &["from: Decimal", "to: Decimal", "Iterator<Item=Decimal>"]);
+
+        let hash = lib.set_fn_3("range", |from, to, step| StepDecimalRange::new(from, to, step));
+        lib.update_fn_metadata(hash, &["from: Decimal", "to: Decimal", "step: Decimal", "Iterator<Item=Decimal>"]);
+    }
 });

src/packages/logic.rs

@@ -3,9 +3,6 @@
 use crate::def_package;
 use crate::plugin::*;
 
-#[cfg(feature = "decimal")]
-use rust_decimal::Decimal;
-
 #[cfg(any(
     not(feature = "no_float"),
     all(not(feature = "only_i32"), not(feature = "only_i64"))

src/result.rs

@@ -153,13 +153,13 @@ impl fmt::Display for EvalAltResult {
 
             #[cfg(not(feature = "no_function"))]
             Self::ErrorInFunctionCall(s, src, err, _) if crate::engine::is_anonymous_fn(s) => {
-                write!(f, "{}, in call to closure", err)?;
+                write!(f, "{} in call to closure", err)?;
                 if !src.is_empty() {
                     write!(f, " @ '{}'", src)?;
                 }
             }
             Self::ErrorInFunctionCall(s, src, err, _) => {
-                write!(f, "{}, in call to function {}", err, s)?;
+                write!(f, "{} in call to function {}", err, s)?;
                 if !src.is_empty() {
                     write!(f, " @ '{}'", src)?;
                 }

tests/for.rs

@@ -2,7 +2,7 @@ use rhai::{Engine, EvalAltResult, Module, INT};
 
 #[cfg(not(feature = "no_index"))]
 #[test]
-fn test_for_array() -> Result<(), Box<EvalAltResult>> {
+fn test_for() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();
 
     let script = r"
@@ -27,6 +27,81 @@ fn test_for_array() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(engine.eval::<INT>(script)?, 35);
 
+    assert_eq!(
+        engine.eval::<INT>(
+            r"
+                let sum = 0;
+                for x in range(1, 10, 2) { sum += x; }
+                sum
+            "
+        )?,
+        25
+    );
+
+    assert_eq!(
+        engine.eval::<INT>(
+            r"
+                let sum = 0;
+                for x in range(10, 1, 2) { sum += x; }
+                sum
+            "
+        )?,
+        0
+    );
+
+    assert_eq!(
+        engine.eval::<INT>(
+            r"
+                let sum = 0;
+                for x in range(1, 10, -2) { sum += x; }
+                sum
+            "
+        )?,
+        0
+    );
+
+    assert_eq!(
+        engine.eval::<INT>(
+            r"
+                let sum = 0;
+                for x in range(10, 1, -2) { sum += x; }
+                sum
+            "
+        )?,
+        30
+    );
+
+    Ok(())
+}
+
+#[cfg(not(feature = "unchecked"))]
+#[test]
+fn test_for_overflow() -> Result<(), Box<EvalAltResult>> {
+    let engine = Engine::new();
+
+    #[cfg(not(feature = "only_i32"))]
+    let script = r"
+        let sum = 0;
+
+        for x in range(9223372036854775807, 0, 9223372036854775807) {
+            sum += 1;
+        }
+
+        sum
+    ";
+    #[cfg(feature = "only_i32")]
+    let script = r"
+        let sum = 0;
+
+        for x in range(2147483647 , 0, 2147483647 ) {
+            sum += 1;
+        }
+
+        sum
+    ";
+
+    assert_eq!(engine.eval::<INT>(script)?, 0);
+
     Ok(())
 }