Fix for a array deserialize panic found during fuzz testing

server/src/address_space/variable.rs

@@ -520,7 +520,7 @@ impl Variable {
                 if self.data_type == DataTypeId::Byte.into() {
                     if let Variant::ByteString(_) = value {
                         // Convert the value from a byte string to a byte array
-                        value = value.to_byte_array();
+                        value = value.to_byte_array()?;
                     }
                 }
             }

server/src/tests/address_space.rs

@@ -473,7 +473,7 @@ fn multi_dimension_array_as_variable() {
     let values = (0..100)
         .map(|i| Variant::Int32(i))
         .collect::<Vec<Variant>>();
-    let mda = Array::new_multi(VariantTypeId::Int32, values, vec![10u32, 10u32]);
+    let mda = Array::new_multi(VariantTypeId::Int32, values, vec![10u32, 10u32]).unwrap();
     assert!(mda.is_valid());
 
     // Get the variable node back from the address space, ensure that the ValueRank and ArrayDimensions are correct

types/fuzz/Cargo.toml

@@ -14,12 +14,15 @@ libfuzzer-sys = "0.4"
 [dependencies.opcua-types]
 path = ".."
 
+[dependencies.opcua-console-logging]
+path = "../../console-logging"
+
 # Prevent this from interfering with workspaces
 [workspace]
 members = ["."]
 
 [[bin]]
-name = "fuzz_target_1"
-path = "fuzz_targets/fuzz_target_1.rs"
+name = "fuzz_deserialize"
+path = "fuzz_targets/fuzz_deserialize.rs"
 test = false
 doc = false

types/fuzz/fuzz_targets/fuzz_target_1.rs → types/fuzz/fuzz_targets/fuzz_deserialize.rs

@@ -11,6 +11,7 @@ pub fn deserialize(data: &[u8], decoding_options: &DecodingOptions) -> Result<Va
 }
 
 fuzz_target!(|data: &[u8]| {
+    opcua_console_logging::init();
     let decoding_options = DecodingOptions::default();
     let _ = deserialize(data, &decoding_options);
 });

types/src/array.rs

@@ -1,4 +1,5 @@
 use crate::variant::*;
+use crate::StatusCode;
 
 /// An array is a vector of values with an optional number of dimensions.
 /// It is expected that the multi-dimensional array is valid, or it might not be encoded or decoded
@@ -19,44 +20,59 @@ pub struct Array {
 }
 
 impl Array {
-    pub fn new_single<V>(value_type: VariantTypeId, values: V) -> Array
+    pub fn new_single<V>(value_type: VariantTypeId, values: V) -> Result<Array, StatusCode>
     where
         V: Into<Vec<Variant>>,
     {
         let values = values.into();
-        Self::validate_array_type_to_values(value_type, &values);
-        Array {
-            value_type,
-            values,
-            dimensions: Vec::new(),
+        if Self::validate_array_type_to_values(value_type, &values) {
+            Ok(Array {
+                value_type,
+                values,
+                dimensions: Vec::new(),
+            })
+        } else {
+            Err(StatusCode::BadDecodingError)
         }
     }
 
-    pub fn new_multi<V, D>(value_type: VariantTypeId, values: V, dimensions: D) -> Array
+    pub fn new_multi<V, D>(
+        value_type: VariantTypeId,
+        values: V,
+        dimensions: D,
+    ) -> Result<Array, StatusCode>
     where
         V: Into<Vec<Variant>>,
         D: Into<Vec<u32>>,
     {
         let values = values.into();
-        Self::validate_array_type_to_values(value_type, &values);
-        Array {
-            value_type,
-            values,
-            dimensions: dimensions.into(),
+        if Self::validate_array_type_to_values(value_type, &values) {
+            Ok(Array {
+                value_type,
+                values,
+                dimensions: dimensions.into(),
+            })
+        } else {
+            Err(StatusCode::BadDecodingError)
         }
     }
 
     /// This is a runtime check to ensure the type of the array also matches the types of the variants in the array.
-    fn validate_array_type_to_values(value_type: VariantTypeId, values: &Vec<Variant>) {
+    fn validate_array_type_to_values(value_type: VariantTypeId, values: &Vec<Variant>) -> bool {
         match value_type {
             VariantTypeId::Array | VariantTypeId::Empty => {
-                panic!("Invalid array type supplied")
+                error!("Invalid array type supplied");
+                false
+            }
+            _ => {
+                if !values_are_of_type(&values, value_type) {
+                    // If the values exist, then validate them to the type
+                    error!("Value type of array does not match contents");
+                    false
+                } else {
+                    true
+                }
             }
-            _ => {}
-        }
-        // If the values exist, then validate them to the type
-        if !values_are_of_type(&values, value_type) {
-            panic!("Value type of array does not match contents");
         }
     }
 

types/src/tests/encoding.rs

@@ -484,7 +484,7 @@ fn argument() {
 
 // test decoding of an null array  null != empty!
 #[test]
-fn null_array() -> EncodingResult<()>{
+fn null_array() -> EncodingResult<()> {
     // @FIXME currently creating an null array via Array or Variant is not possible so do it by hand
     let vec = Vec::new();
     let mut stream = Cursor::new(vec);
@@ -495,6 +495,13 @@ fn null_array() -> EncodingResult<()>{
     let actual = stream.into_inner();
     let mut stream = Cursor::new(actual);
     let arr = Variant::decode(&mut stream, &DecodingOptions::default())?;
-    assert_eq!(arr, Variant::Array(Box::new(Array{ value_type: VariantTypeId::Boolean, values: Vec::new(), dimensions: Vec::new()})));
+    assert_eq!(
+        arr,
+        Variant::Array(Box::new(Array {
+            value_type: VariantTypeId::Boolean,
+            values: Vec::new(),
+            dimensions: Vec::new()
+        }))
+    );
     Ok(())
-}
+}

types/src/tests/variant.rs

@@ -1441,7 +1441,7 @@ fn variant_bytestring_to_bytearray() {
     let v = ByteString::from(&[0x1, 0x2, 0x3, 0x4]);
     let v = Variant::from(v);
 
-    let v = v.to_byte_array();
+    let v = v.to_byte_array().unwrap();
     assert_eq!(v.array_data_type().unwrap(), DataTypeId::Byte.into());
 
     let array = match v {