refactor(core): Don't unwrap an ExtensionObject if it contains a builtin

We had fuzzer issuers from this where the encoding != decoding because
we would not wrap the builtin content again.

src/ua_types_encoding_json.c

@@ -2209,6 +2209,17 @@ getArrayUnwrapType(ParseCtx *ctx, size_t arrayIndex) {
         return NULL;
     }
 
+    /* The content is a builtin type that could have been directly encoded in
+     * the Variant, there was no need to wrap in an ExtensionObject. But this
+     * means for us, that somebody made an extra effort to explicitly get an
+     * ExtensionObject. So we keep it. As an added advantage we will generate
+     * the same JSON again when encoding again. */
+    UA_Boolean isBuiltin = (typeOfBody->typeKind <= UA_DATATYPEKIND_DIAGNOSTICINFO);
+    if(isBuiltin) {
+        ctx->index = oldIndex; /* Restore the index */
+        return NULL;
+    }
+
     /* Get the typeId index for faster comparison below.
      * Cannot fail as getExtensionObjectType already looked this up. */
     size_t typeIdIndex = 0;
@@ -2377,7 +2388,8 @@ DECODE_JSON(Variant) {
             {UA_JSONKEY_DIMENSION, &dst->arrayDimensions, VariantDimension_decodeJson, false, NULL}
         };
 
-        /* Can we unwrap ExtensionObjects in the array? */
+        /* Try to unwrap ExtensionObjects in the array.
+         * The members must all have the same type. */
         if(dst->type == &UA_TYPES[UA_TYPES_EXTENSIONOBJECT]) {
             const UA_DataType *unwrapType = getArrayUnwrapType(ctx, bodyIndex);
             if(unwrapType) {
@@ -2534,22 +2546,34 @@ Variant_decodeJsonUnwrapExtensionObject(ParseCtx *ctx, void *p, const UA_DataTyp
     }
 
     /* The content is still encoded, cannot unwrap */
-    if(eo.encoding != UA_EXTENSIONOBJECT_DECODED) {
-        /* Decode as ExtensionObject */
-        dst->data = UA_new(&UA_TYPES[UA_TYPES_EXTENSIONOBJECT]);
-        if(!dst->data) {
-            UA_ExtensionObject_clear(&eo);
-            return UA_STATUSCODE_BADOUTOFMEMORY;
-        }
-        dst->type = &UA_TYPES[UA_TYPES_EXTENSIONOBJECT];
-        *(UA_ExtensionObject*)dst->data = eo;
-        return UA_STATUSCODE_GOOD;
-    }
+    if(eo.encoding != UA_EXTENSIONOBJECT_DECODED)
+        goto use_eo;
+
+    /* The content is a builtin type that could have been directly encoded in
+     * the Variant, there was no need to wrap in an ExtensionObject. But this
+     * means for us, that somebody made an extra effort to explicitly get an
+     * ExtensionObject. So we keep it. As an added advantage we will generate
+     * the same JSON again when encoding again. */
+    UA_Boolean isBuiltin =
+        (eo.content.decoded.type->typeKind <= UA_DATATYPEKIND_DIAGNOSTICINFO);
+    if(isBuiltin)
+        goto use_eo;
 
     /* Unwrap the ExtensionObject */
     dst->data = eo.content.decoded.data;
     dst->type = eo.content.decoded.type;
     return UA_STATUSCODE_GOOD;
+
+ use_eo:
+    /* Don't unwrap */
+    dst->data = UA_new(&UA_TYPES[UA_TYPES_EXTENSIONOBJECT]);
+    if(!dst->data) {
+        UA_ExtensionObject_clear(&eo);
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+    }
+    dst->type = &UA_TYPES[UA_TYPES_EXTENSIONOBJECT];
+    *(UA_ExtensionObject*)dst->data = eo;
+    return UA_STATUSCODE_GOOD;
 }
 
 status

tests/check_types_builtin_json.c

@@ -5175,16 +5175,11 @@ START_TEST(UA_Variant_ExtensionObjectArray_json_decode) {
 
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT], NULL);
 
-    UA_Boolean *testArray;
-    testArray = (UA_Boolean*)(out.data);
-    // then
+    // don't unwrap builtin types that shouldn't be wrapped in the first place
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
-    //decoded as False
-    ck_assert_int_eq(testArray[0], 0);
-    ck_assert_int_eq(testArray[1], 1);
+    ck_assert_int_eq(out.type->typeKind, UA_DATATYPEKIND_EXTENSIONOBJECT);
     ck_assert_uint_eq(out.arrayDimensionsSize, 0);
     ck_assert_uint_eq(out.arrayLength, 2);
-    ck_assert_int_eq(out.type->typeKind, UA_DATATYPEKIND_BOOLEAN);
     UA_Variant_clear(&out);
 }
 END_TEST