Repository: mitsuhiko/minijinja (rust)

Issue:
`ops::int_div` panics on overflow
## Description

`ops::int_div` panics on overflow.

## Reproduction steps

1. Add a test case below into the mod tests block in minijinja/src/value/ops.rs.

```
    #[test]
    #[should_panic]
    fn test_int_div_overflow() {
        _ = int_div(&Value::from(i128::MIN), &Value::from(-1i128));
    }
```

2. Run `cargo test -- test_int_div_overflow`
3. It prints the following output:

```
running 1 test
test value::ops::tests::test_int_div_overflow - should panic ... ok
```

Additional helpful information:

- Version of minijinja: the latest commit https://github.com/mitsuhiko/minijinja/commit/ae9017ebe881374b329d0c821a2586621087d54a
- Version of rustc: 1.72.0 (5680fa18f 2023-08-23)
- Operating system and version: Ubuntu 22.04

## What did you expect

Should return `Err(failed_op("//", lhs, rhs))`.
I suppose we should use `i128::checked_div_euclid` instead of `i128::div_euclid` to check an overflow.


Pull request:
Fix an integer overflow on division/abs and neg
Fixes #345

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 162b157..3bf5c0e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 All notable changes to MiniJinja are documented here.
 
+## 1.0.8
+
+- Fixed a few overflow panics: dividing integers with an overflow and
+  related overflows in the `abs` and `neg` filter.  #347
+
 ## 1.0.7
 
 - Added support for `keep_trailing_newlines` which allows you to disable
diff --git a/minijinja/Cargo.toml b/minijinja/Cargo.toml
index 73fa593..77d6e64 100644
--- a/minijinja/Cargo.toml
+++ b/minijinja/Cargo.toml
@@ -17,7 +17,14 @@ features = ["loader", "json", "urlencode", "custom_syntax", "fuel"]
 rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "doc-header.html"]
 
 [features]
-default = ["builtins", "debug", "deserialization", "macros", "multi_template", "adjacent_loop_items"]
+default = [
+    "builtins",
+    "debug",
+    "deserialization",
+    "macros",
+    "multi_template",
+    "adjacent_loop_items",
+]
 
 # API features
 preserve_order = ["indexmap"]
@@ -60,7 +67,7 @@ unicode-ident = { version = "1.0.5", optional = true }
 unicase = { version = "2.6.0", optional = true }
 
 [dev-dependencies]
-insta = { version = "1.26.0", features = ["glob", "serde"] }
+insta = { version = "1.31.0", features = ["glob", "serde"] }
 serde = { version = "1.0.130", features = ["derive"] }
 serde_json = "1.0.68"
 similar-asserts = "1.4.2"
diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs
index a201ff0..75ad46a 100644
--- a/minijinja/src/filters.rs
+++ b/minijinja/src/filters.rs
@@ -552,8 +552,15 @@ mod builtins {
     #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
     pub fn abs(value: Value) -> Result<Value, Error> {
         match value.0 {
-            ValueRepr::I64(x) => Ok(Value::from(x.abs())),
-            ValueRepr::I128(x) => Ok(Value::from(x.0.abs())),
+            ValueRepr::I64(x) => match x.checked_abs() {
+                Some(rv) => Ok(Value::from(rv)),
+                None => Ok(Value::from((x as i128).abs())), // this cannot overflow
+            },
+            ValueRepr::I128(x) => {
+                x.0.checked_abs()
+                    .map(Value::from)
+                    .ok_or_else(|| Error::new(ErrorKind::InvalidOperation, "overflow on abs"))
+            }
             ValueRepr::F64(x) => Ok(Value::from(x.abs())),
             _ => Err(Error::new(
                 ErrorKind::InvalidOperation,
diff --git a/minijinja/src/value/ops.rs b/minijinja/src/value/ops.rs
index fc1280d..9610d95 100644
--- a/minijinja/src/value/ops.rs
+++ b/minijinja/src/value/ops.rs
@@ -204,7 +204,9 @@ pub fn int_div(lhs: &Value, rhs: &Value) -> Result<Value, Error> {
     match coerce(lhs, rhs) {
         Some(CoerceResult::I128(a, b)) => {
             if b != 0 {
-                Ok(int_as_value(a.div_euclid(b)))
+                a.checked_div_euclid(b)
+                    .ok_or_else(|| failed_op("//", lhs, rhs))
+                    .map(int_as_value)
             } else {
                 Err(failed_op("//", lhs, rhs))
             }
@@ -235,7 +237,9 @@ pub fn neg(val: &Value) -> Result<Value, Error> {
             ValueRepr::F64(x) => Ok((-x).into()),
             _ => {
                 if let Ok(x) = i128::try_from(val.clone()) {
-                    Ok(int_as_value(-x))
+                    x.checked_mul(-1)
+                        .ok_or_else(|| Error::new(ErrorKind::InvalidOperation, "overflow"))
+                        .map(int_as_value)
                 } else {
                     Err(Error::from(ErrorKind::InvalidOperation))
                 }
@@ -283,6 +287,12 @@ mod tests {
 
     use similar_asserts::assert_eq;
 
+    #[test]
+    fn test_neg() {
+        let err = neg(&Value::from(i128::MIN)).unwrap_err();
+        assert_eq!(err.to_string(), "invalid operation: overflow");
+    }
+
     #[test]
     fn test_adding() {
         let err = add(&Value::from("a"), &Value::from(42)).unwrap_err();
@@ -339,6 +349,12 @@ mod tests {
             div(&Value::from(100), &Value::from(2)).unwrap(),
             Value::from(50.0)
         );
+
+        let err = int_div(&Value::from(i128::MIN), &Value::from(-1i128)).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "invalid operation: unable to calculate -170141183460469231731687303715884105728 // -1"
+        );
     }
 
     #[test]


Test patch:
diff --git a/minijinja/tests/test_filters.rs b/minijinja/tests/test_filters.rs
index ebdf5f6..d6dd232 100644
--- a/minijinja/tests/test_filters.rs
+++ b/minijinja/tests/test_filters.rs
@@ -2,7 +2,7 @@ use minijinja::value::Value;
 use minijinja::{args, Environment};
 use similar_asserts::assert_eq;
 
-use minijinja::filters::indent;
+use minijinja::filters::{abs, indent};
 
 #[test]
 fn test_filter_with_non() {
@@ -77,3 +77,11 @@ fn test_indent_with_all_indented() {
         String::from("  test\n  test1\n  \n  test2")
     );
 }
+
+#[test]
+fn test_abs_overflow() {
+    let ok = abs(Value::from(i64::MIN)).unwrap();
+    assert_eq!(ok, Value::from(-(i64::MIN as i128)));
+    let err = abs(Value::from(i128::MIN)).unwrap_err();
+    assert_eq!(err.to_string(), "invalid operation: overflow on abs");
+}