fix!: check abi integer input is within signed range (#7316)

Co-authored-by: Michael J Klein <michaeljklein@users.noreply.github.com>

test_programs/execution_success/signed_cmp/Prover.toml

@@ -1 +1 @@
-minus_one = 255
+minus_one = -1

test_programs/execution_success/signed_div/Prover.toml

@@ -1,51 +1,51 @@
 [[ops]]
 lhs = 4
-rhs = 255    # -1
-result = 252 # -4
+rhs = -1
+result = -4
 
 [[ops]]
 lhs = 4
-rhs = 254    # -2
-result = 254 # -2
+rhs = -2
+result = -2
 
 [[ops]]
 lhs = 4
-rhs = 253    # -3
-result = 255 # -1
+rhs = -3
+result = -1
 
 [[ops]]
 lhs = 4
-rhs = 252    # -4
-result = 255 # -1
+rhs = -4
+result = -1
 
 [[ops]]
 lhs = 4
-rhs = 251  # -5
+rhs = -5
 result = 0
 
 [[ops]]
-lhs = 252  # -4
-rhs = 255  # -1
+lhs = -4
+rhs = -1
 result = 4
 
 [[ops]]
-lhs = 252  # -4
-rhs = 254  # -2
+lhs = -4
+rhs = -2
 result = 2
 
 [[ops]]
-lhs = 252  # -4
-rhs = 253  # -3
+lhs = -4
+rhs = -3
 result = 1
 
 [[ops]]
-lhs = 252  # -4
-rhs = 252  # -4
+lhs = -4
+rhs = -4
 result = 1
 
 [[ops]]
-lhs = 252  # -4
-rhs = 251  # -5
+lhs = -4
+rhs = -5
 result = 0
 
 

tooling/noir_js/test/node/cjs.test.cjs

@@ -59,21 +59,21 @@ it('0x prefixed string input for inputs will throw', async () => {
 });
 
 describe('input validation', () => {
-  it('x should be a uint64 not a string', async () => {
+  it('x should be a int64 not a string', async () => {
     const inputs = {
       x: 'foo',
       y: '3',
     };
 
     try {
       await new Noir(assert_lt_json).execute(inputs);
-      chai.expect.fail('Expected generatedWitness to throw, due to x not being convertible to a uint64');
+      chai.expect.fail('Expected generatedWitness to throw, due to x not being convertible to a int64');
     } catch (error) {
       const knownError = error;
       chai
         .expect(knownError.message)
         .to.equal(
-          'Expected witness values to be integers, provided value causes `invalid digit found in string` error',
+          'The value passed for parameter `x` is invalid:\nExpected witness values to be integers, but `foo` failed with `invalid digit found in string`',
         );
     }
   });

tooling/noir_js/test/node/smoke.test.ts

@@ -58,19 +58,19 @@ it('0x prefixed string input for inputs will throw', async () => {
 });
 
 describe('input validation', () => {
-  it('x should be a uint64 not a string', async () => {
+  it('x should be a int64 not a string', async () => {
     const inputs = {
       x: 'foo',
       y: '3',
     };
 
     try {
       await new Noir(assert_lt_program).execute(inputs);
-      expect.fail('Expected generatedWitness to throw, due to x not being convertible to a uint64');
+      expect.fail('Expected generatedWitness to throw, due to x not being convertible to a int64');
     } catch (error) {
       const knownError = error as Error;
       expect(knownError.message).to.equal(
-        'Expected witness values to be integers, provided value causes `invalid digit found in string` error',
+        'The value passed for parameter `x` is invalid:\nExpected witness values to be integers, but `foo` failed with `invalid digit found in string`',
       );
     }
   });

tooling/noirc_abi/src/errors.rs

@@ -2,17 +2,26 @@ use crate::{
     input_parser::{InputTypecheckingError, InputValue},
     AbiType,
 };
-use acvm::acir::native_types::Witness;
+use acvm::{acir::native_types::Witness, AcirField, FieldElement};
 use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub enum InputParserError {
     #[error("input file is badly formed, could not parse, {0}")]
     ParseInputMap(String),
-    #[error("Expected witness values to be integers, provided value causes `{0}` error")]
-    ParseStr(String),
-    #[error("Could not parse hex value {0}")]
-    ParseHexStr(String),
+    #[error(
+        "The value passed for parameter `{arg_name}` is invalid:\nExpected witness values to be integers, but `{value}` failed with `{error}`"
+    )]
+    ParseStr { arg_name: String, value: String, error: String },
+    #[error("The value passed for parameter `{arg_name}` is invalid:\nValue {value} is less than minimum allowed value of {min}")]
+    InputUnderflowsMinimum { arg_name: String, value: String, min: String },
+    #[error("The value passed for parameter `{arg_name}` is invalid:\nValue {value} exceeds maximum allowed value of {max}")]
+    InputOverflowsMaximum { arg_name: String, value: String, max: String },
+    #[error(
+        "The value passed for parameter `{arg_name}` is invalid:\nValue {value} exceeds field modulus. Values must fall within [0, {})",
+        FieldElement::modulus()
+    )]
+    InputExceedsFieldModulus { arg_name: String, value: String },
     #[error("cannot parse value into {0:?}")]
     AbiTypeMismatch(AbiType),
     #[error("Expected argument `{0}`, but none was found")]

tooling/noirc_abi/src/input_parser/json.rs

@@ -1,4 +1,7 @@
-use super::{field_to_signed_hex, parse_str_to_field, parse_str_to_signed, InputValue};
+use super::{
+    field_to_signed_hex, parse_integer_to_signed, parse_str_to_field, parse_str_to_signed,
+    InputValue,
+};
 use crate::{errors::InputParserError, Abi, AbiType, MAIN_RETURN_NAME};
 use acvm::{AcirField, FieldElement};
 use iter_extended::{try_btree_map, try_vecmap};
@@ -69,9 +72,9 @@ pub enum JsonTypes {
     // Just a regular integer, that can fit in 64 bits.
     //
     // The JSON spec does not specify any limit on the size of integer number types,
-    // however we restrict the allowable size. Values which do not fit in a u64 should be passed
+    // however we restrict the allowable size. Values which do not fit in a i64 should be passed
     // as a string.
-    Integer(u64),
+    Integer(i64),
     // Simple boolean flag
     Bool(bool),
     // Array of JsonTypes
@@ -147,12 +150,20 @@ impl InputValue {
         let input_value = match (value, param_type) {
             (JsonTypes::String(string), AbiType::String { .. }) => InputValue::String(string),
             (JsonTypes::String(string), AbiType::Integer { sign: crate::Sign::Signed, width }) => {
-                InputValue::Field(parse_str_to_signed(&string, *width)?)
+                InputValue::Field(parse_str_to_signed(&string, *width, arg_name)?)
             }
             (
                 JsonTypes::String(string),
                 AbiType::Field | AbiType::Integer { .. } | AbiType::Boolean,
-            ) => InputValue::Field(parse_str_to_field(&string)?),
+            ) => InputValue::Field(parse_str_to_field(&string, arg_name)?),
+
+            (
+                JsonTypes::Integer(integer),
+                AbiType::Integer { sign: crate::Sign::Signed, width },
+            ) => {
+                let new_value = parse_integer_to_signed(integer as i128, *width, arg_name)?;
+                InputValue::Field(new_value)
+            }
 
             (
                 JsonTypes::Integer(integer),
@@ -166,8 +177,13 @@ impl InputValue {
             (JsonTypes::Bool(boolean), AbiType::Boolean) => InputValue::Field(boolean.into()),
 
             (JsonTypes::Array(array), AbiType::Array { typ, .. }) => {
-                let array_elements =
-                    try_vecmap(array, |value| InputValue::try_from_json(value, typ, arg_name))?;
+                let mut index = 0;
+                let array_elements = try_vecmap(array, |value| {
+                    let sub_name = format!("{arg_name}[{index}]");
+                    let value = InputValue::try_from_json(value, typ, &sub_name);
+                    index += 1;
+                    value
+                })?;
                 InputValue::Vec(array_elements)
             }
 
@@ -186,8 +202,12 @@ impl InputValue {
             }
 
             (JsonTypes::Array(array), AbiType::Tuple { fields }) => {
+                let mut index = 0;
                 let tuple_fields = try_vecmap(array.into_iter().zip(fields), |(value, typ)| {
-                    InputValue::try_from_json(value, typ, arg_name)
+                    let sub_name = format!("{arg_name}[{index}]");
+                    let value = InputValue::try_from_json(value, typ, &sub_name);
+                    index += 1;
+                    value
                 })?;
                 InputValue::Vec(tuple_fields)
             }
@@ -201,11 +221,13 @@ impl InputValue {
 
 #[cfg(test)]
 mod test {
+    use acvm::FieldElement;
     use proptest::prelude::*;
 
     use crate::{
         arbitrary::arb_abi_and_input_map,
         input_parser::{arbitrary::arb_signed_integer_type_and_value, json::JsonTypes, InputValue},
+        AbiType,
     };
 
     use super::{parse_json, serialize_to_json};
@@ -234,4 +256,26 @@ mod test {
             prop_assert_eq!(output_number, value);
         }
     }
+
+    #[test]
+    fn errors_on_integer_to_signed_integer_overflow() {
+        let typ = AbiType::Integer { sign: crate::Sign::Signed, width: 8 };
+        let input = JsonTypes::Integer(128);
+        assert!(InputValue::try_from_json(input, &typ, "foo").is_err());
+
+        let typ = AbiType::Integer { sign: crate::Sign::Signed, width: 16 };
+        let input = JsonTypes::Integer(32768);
+        assert!(InputValue::try_from_json(input, &typ, "foo").is_err());
+    }
+
+    #[test]
+    fn try_from_json_negative_integer() {
+        let typ = AbiType::Integer { sign: crate::Sign::Signed, width: 8 };
+        let input = JsonTypes::Integer(-1);
+        let InputValue::Field(field) = InputValue::try_from_json(input, &typ, "foo").unwrap()
+        else {
+            panic!("Expected field");
+        };
+        assert_eq!(field, FieldElement::from(255_u128));
+    }
 }