fix(core): Fix an oss-fuzz issue for JSON variant decoding

src/ua_types_encoding_json.c

@@ -1567,6 +1567,18 @@ UA_calcSizeJson(const void *src, const UA_DataType *type,
 /* decode without moving the token index */
 #define DECODE_DIRECT_JSON(DST, TYPE) TYPE##_decodeJson((UA_##TYPE*)DST, NULL, ctx, parseCtx, false)
 
+/* If parseCtx->index points to the beginning of an object, move the index to
+ * the next token after this object. Attention! The index can be moved after the
+ * last parsed token. So the array length has to be checked afterwards. */
+static void
+skipObject(ParseCtx *parseCtx) {
+    int end = parseCtx->tokenArray[parseCtx->index].end;
+    do {
+        parseCtx->index++;
+    } while(parseCtx->index < parseCtx->tokenCount &&
+            parseCtx->tokenArray[parseCtx->index].start < end);
+}
+
 static status
 Array_decodeJson(void *dst, const UA_DataType *type, CtxJson *ctx, 
         ParseCtx *parseCtx, UA_Boolean moveToken);
@@ -1587,14 +1599,6 @@ getJsmnType(const ParseCtx *parseCtx) {
     return parseCtx->tokenArray[parseCtx->index].type;
 }
 
-static UA_Boolean
-isJsonTokenNull(const CtxJson *ctx, jsmntok_t *token) {
-    if(token->type != JSMN_PRIMITIVE)
-        return false;
-    char* elem = (char*)(ctx->pos + token->start);
-    return (elem[0] == 'n' && elem[1] == 'u' && elem[2] == 'l' && elem[3] == 'l');
-}
-
 UA_Boolean
 isJsonNull(const CtxJson *ctx, const ParseCtx *parseCtx) {
     if(parseCtx->index >= parseCtx->tokenCount)
@@ -2613,8 +2617,10 @@ DECODE_JSON(Variant) {
     /* First search for the variant type in the json object. */
     size_t searchResultType = 0;
     status ret = lookAheadForKey(UA_JSONKEY_TYPE, ctx, parseCtx, &searchResultType);
-    if(ret != UA_STATUSCODE_GOOD)
-        return UA_STATUSCODE_BADDECODINGERROR;
+    if(ret != UA_STATUSCODE_GOOD) {
+        skipObject(parseCtx);
+        return UA_STATUSCODE_GOOD;
+    }
 
     size_t size = (size_t)(parseCtx->tokenArray[searchResultType].end -
                            parseCtx->tokenArray[searchResultType].start);
@@ -2623,24 +2629,25 @@ DECODE_JSON(Variant) {
     if(size < 1 || parseCtx->tokenArray[searchResultType].type != JSMN_PRIMITIVE)
         return UA_STATUSCODE_BADDECODINGERROR;
     
-    /*Parse the type*/
+    /* Parse the type */
     UA_UInt64 idTypeDecoded = 0;
     char *idTypeEncoded = (char*)(ctx->pos + parseCtx->tokenArray[searchResultType].start);
     status typeDecodeStatus = atoiUnsigned(idTypeEncoded, size, &idTypeDecoded);
-    
-    /* value is not a valid number */
     if(typeDecodeStatus != UA_STATUSCODE_GOOD)
         return typeDecodeStatus;
 
-    /*Set the type, Get the Type by nodeID!*/
+    /* A NULL Variant */
+    if(idTypeDecoded == 0) {
+        skipObject(parseCtx);
+        return UA_STATUSCODE_GOOD;
+    }
+
+    /* Set the type */
     UA_NodeId typeNodeId = UA_NODEID_NUMERIC(0, (UA_UInt32)idTypeDecoded);
-    const UA_DataType *bodyType = UA_findDataType(&typeNodeId);
-    if(!bodyType)
+    dst->type = UA_findDataType(&typeNodeId);
+    if(!dst->type)
         return UA_STATUSCODE_BADDECODINGERROR;
     
-    /*Set the type*/
-    dst->type = bodyType;
-    
     /* Search for body */
     size_t searchResultBody = 0;
     ret = lookAheadForKey(UA_JSONKEY_BODY, ctx, parseCtx, &searchResultBody);
@@ -2649,19 +2656,9 @@ DECODE_JSON(Variant) {
         return UA_STATUSCODE_BADDECODINGERROR;
     }
 
-    /* get body token */
-    jsmntok_t bodyToken = parseCtx->tokenArray[searchResultBody];
-
-    /*BODY is null?*/
-    UA_Boolean isBodyNull = false;
-    if(isJsonTokenNull(ctx, &bodyToken)) {
-        dst->data = NULL;
-        isBodyNull = true;
-    }
-
     /* value is an array? */
     UA_Boolean isArray = false;
-    if(bodyToken.type == JSMN_ARRAY) {
+    if(parseCtx->tokenArray[searchResultBody].type == JSMN_ARRAY) {
         isArray = true;
         dst->arrayLength = (size_t)parseCtx->tokenArray[searchResultBody].size;
     }
@@ -2680,13 +2677,13 @@ DECODE_JSON(Variant) {
         return UA_STATUSCODE_BADDECODINGERROR;
     
     /* Get the datatype of the content. The type must be a builtin data type.
-    * All not-builtin types are wrapped in an ExtensionObject. */
-    if(bodyType->typeKind > UA_TYPES_DIAGNOSTICINFO)
+     * All not-builtin types are wrapped in an ExtensionObject. */
+    if(dst->type->typeKind > UA_TYPES_DIAGNOSTICINFO)
         return UA_STATUSCODE_BADDECODINGERROR;
 
     /* A variant cannot contain a variant. But it can contain an array of
-        * variants */
-    if(bodyType->typeKind == UA_DATATYPEKIND_VARIANT && !isArray)
+     * variants */
+    if(dst->type->typeKind == UA_DATATYPEKIND_VARIANT && !isArray)
         return UA_STATUSCODE_BADDECODINGERROR;
     
     /* Decode an array */
@@ -2697,33 +2694,30 @@ DECODE_JSON(Variant) {
             {UA_JSONKEY_DIMENSION, &dst->arrayDimensions,
              (decodeJsonSignature) VariantDimension_decodeJson, false, NULL}};
         if(!hasDimension) {
-            ret = decodeFields(ctx, parseCtx, entries, 2, bodyType); /*use first 2 fields*/
+            ret = decodeFields(ctx, parseCtx, entries, 2, dst->type); /*use first 2 fields*/
         } else {
-            ret = decodeFields(ctx, parseCtx, entries, 3, bodyType); /*use all fields*/
+            ret = decodeFields(ctx, parseCtx, entries, 3, dst->type); /*use all fields*/
         }      
         return ret;
     }
 
     /* Decode a value wrapped in an ExtensionObject */
-    if(bodyType->typeKind == UA_DATATYPEKIND_EXTENSIONOBJECT) {
+    if(dst->type->typeKind == UA_DATATYPEKIND_EXTENSIONOBJECT) {
         DecodeEntry entries[2] =
             {{UA_JSONKEY_TYPE, NULL, NULL, false, NULL},
              {UA_JSONKEY_BODY, dst,
               (decodeJsonSignature)Variant_decodeJsonUnwrapExtensionObject, false, NULL}};
-        return decodeFields(ctx, parseCtx, entries, 2, bodyType);
+        return decodeFields(ctx, parseCtx, entries, 2, dst->type);
     }
 
     /* Allocate Memory for Body */
-    if(!isBodyNull) {
-        dst->data = UA_new(bodyType);
-        if(!dst->data)
-            return UA_STATUSCODE_BADOUTOFMEMORY;
-    }
-
+    dst->data = UA_new(dst->type);
+    if(!dst->data)
+        return UA_STATUSCODE_BADOUTOFMEMORY;
     DecodeEntry entries[2] =
         {{UA_JSONKEY_TYPE, NULL, NULL, false, NULL},
          {UA_JSONKEY_BODY, dst->data, (decodeJsonSignature) decodeJsonInternal, false, NULL}};
-    return decodeFields(ctx, parseCtx, entries, 2, bodyType);
+    return decodeFields(ctx, parseCtx, entries, 2, dst->type);
 }
 
 DECODE_JSON(DataValue) {

tests/check_types_builtin_json.c

@@ -4348,14 +4348,12 @@ START_TEST(UA_ByteString_null_json_decode) {
     UA_Variant out;
     UA_Variant_init(&out);
     UA_ByteString buf = UA_STRING("{\"Type\":15,\"Body\":null}");
-    // when
-    
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-    // then
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
     ck_assert_int_eq(out.type->typeIndex, UA_TYPES_BYTESTRING);
-    ck_assert_ptr_eq(out.data, NULL);
-    
+    UA_ByteString *outData = (UA_ByteString*)out.data;
+    ck_assert_ptr_ne(outData, NULL);
+    ck_assert_ptr_eq(outData->data, NULL);
     UA_Variant_deleteMembers(&out);
 }
 END_TEST
@@ -4577,26 +4575,18 @@ START_TEST(UA_QualifiedName_json_decode) {
 }
 END_TEST
 
-
 START_TEST(UA_QualifiedName_null_json_decode) {
-    
     UA_Variant out;
     UA_Variant_init(&out);
     UA_ByteString buf = UA_STRING("{\"Type\":20,\"Body\":null}");
-    // when
-    
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-    // then
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
     ck_assert_int_eq(out.type->typeIndex, UA_TYPES_QUALIFIEDNAME);
-    ck_assert_ptr_eq(out.data, NULL);
-    
+    ck_assert_ptr_ne(out.data, NULL);
     UA_Variant_deleteMembers(&out);
 }
 END_TEST
 
-
-/* --------LocalizedText------------ */
 START_TEST(UA_LocalizedText_json_decode) {
     // given
     UA_LocalizedText out;
@@ -4637,17 +4627,13 @@ START_TEST(UA_LocalizedText_missing_json_decode) {
 END_TEST
 
 START_TEST(UA_LocalizedText_null_json_decode) {
-    
     UA_Variant out;
     UA_Variant_init(&out);
     UA_ByteString buf = UA_STRING("{\"Type\":21,\"Body\":null}");
-    // when
-    
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-    // then
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
     ck_assert_int_eq(out.type->typeIndex, UA_TYPES_LOCALIZEDTEXT);
-    ck_assert_ptr_eq(out.data, NULL);
+    ck_assert_ptr_ne(out.data, NULL);
     UA_Variant_deleteMembers(&out);
 }
 END_TEST
@@ -4986,26 +4972,19 @@ START_TEST(UA_DiagnosticInfo_json_decode) {
 END_TEST
 
 START_TEST(UA_DiagnosticInfo_null_json_decode) {
-    
     UA_Variant out;
     UA_Variant_init(&out);
     UA_ByteString buf = UA_STRING("{\"Type\":25,\"Body\":null}");
-    // when
-    
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-    // then
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
     ck_assert_int_eq(out.type->typeIndex, UA_TYPES_DIAGNOSTICINFO);
-    ck_assert_ptr_eq(out.data, NULL);
-    
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasAdditionalInfo, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasInnerDiagnosticInfo, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasInnerStatusCode, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasLocale, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasLocalizedText, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasNamespaceUri, 0);
-    //ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasSymbolicId, 0);
-    
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasAdditionalInfo, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasInnerDiagnosticInfo, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasInnerStatusCode, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasLocale, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasLocalizedText, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasNamespaceUri, 0);
+    ck_assert_uint_eq(((UA_DiagnosticInfo*)out.data)->hasSymbolicId, 0);
     UA_Variant_deleteMembers(&out);
 }
 END_TEST
@@ -5064,20 +5043,12 @@ START_TEST(UA_DataValueMissingFields_json_decode) {
 END_TEST
 
 START_TEST(UA_DataValue_null_json_decode) {
-    // given
-    
     UA_Variant out;
     UA_Variant_init(&out);
     UA_ByteString buf = UA_STRING("{\"Type\":23,\"Body\":null}");
-
-    // when
-    
     UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-    //UA_DiagnosticInfo inner = *out.innerDiagnosticInfo;
-
-    // then
     ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
-    ck_assert_ptr_eq(out.data, NULL);
+    ck_assert_ptr_ne(out.data, NULL);
     UA_Variant_deleteMembers(&out);
 }
 END_TEST
@@ -5213,6 +5184,15 @@ START_TEST(UA_VariantBoolNull_json_decode) {
 }
 END_TEST
 
+START_TEST(UA_VariantNull_json_decode) {
+    UA_Variant out;
+    UA_Variant_init(&out);
+    UA_ByteString buf = UA_STRING("{\"Type\":0}");
+    UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
+    ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
+}
+END_TEST
+
 START_TEST(UA_VariantStringArray_json_decode) {
     // given
     
@@ -5601,34 +5581,14 @@ START_TEST(UA_Variant_Bad_Type2_decode) {
 }
 END_TEST
 
-START_TEST(UA_Variant_Null_decode) {
-    for(int i = 0; i < 100; i++){
-        UA_Variant out;
-        UA_Variant_init(&out);
-        char str[80];
-        sprintf(str, "{\"Type\":%d, \"Body:\":null}", i);
-        UA_ByteString buf = UA_STRING(str);
-        // when
-
-        UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-        // then
-        ck_assert_int_eq(retval, retval);
-        UA_Variant_deleteMembers(&out);
-    }
-}
-END_TEST
-
 START_TEST(UA_Variant_Malformed_decode) {
-    for(int i = 0; i < 100; i++){
+    for(int i = 1; i < 100; i++) {
         UA_Variant out;
         UA_Variant_init(&out);
         char str[80];
         sprintf(str, "{\"Type\":%d, \"Body:\"}", i);
         UA_ByteString buf = UA_STRING(str);
-        // when
-
         UA_StatusCode retval = UA_decodeJson(&buf, &out, &UA_TYPES[UA_TYPES_VARIANT]);
-        // then
         ck_assert_int_eq(retval, UA_STATUSCODE_BADDECODINGERROR);
         UA_Variant_deleteMembers(&out);
     }
@@ -5988,6 +5948,7 @@ static Suite *testSuite_builtin_json(void) {
     //Variant
     tcase_add_test(tc_json_decode, UA_VariantBool_json_decode);
     tcase_add_test(tc_json_decode, UA_VariantBoolNull_json_decode);
+    tcase_add_test(tc_json_decode, UA_VariantNull_json_decode);
     tcase_add_test(tc_json_decode, UA_VariantStringArray_json_decode);
     tcase_add_test(tc_json_decode, UA_VariantStringArrayNull_json_decode);
     tcase_add_test(tc_json_decode, UA_VariantLocalizedTextArrayNull_json_decode);
@@ -6025,13 +5986,9 @@ static Suite *testSuite_builtin_json(void) {
     tcase_add_test(tc_json_decode, UA_Variant_Bad_Type_decode);
     tcase_add_test(tc_json_decode, UA_Variant_Bad_Type2_decode);
 
-    tcase_add_test(tc_json_decode, UA_Variant_Null_decode);
-
-
     tcase_add_test(tc_json_decode, UA_Variant_Malformed_decode);
     tcase_add_test(tc_json_decode, UA_Variant_Malformed2_decode);
 
-
     suite_add_tcase(s, tc_json_decode);
     
     TCase *tc_json_helper = tcase_create("json_helper");