Correctly detect missing key in a key-value pair

The following invalid JSON document would be classified as valid JSON without this change: ``` { "key1":"val1", { "key2":"val2" } } ``` The issue was reported here - #165. Signed-off-by: Gaurav Aggarwal <[email protected]>
FreeRTOS · AniruddhaKanhere · Jul 1, 2024 · Jul 1, 2024 · Jul 1, 2024 · Jul 1, 2024
commit 2cfc19d46c3ca0c320c9c56509603ee91f15c32a
diff --git a/loop_invariants.patch b/loop_invariants.patch
@@ -123,7 +123,7 @@ index 901b2e1..8bdd89c 100644
      {
          if( skipAnyScalar( buf, &i, max ) != true )
          {
-@@ -986,6 +1037,13 @@ static void skipObjectScalars( const char * buf,
+@@ -986,6 +1037,13 @@ static bool skipObjectScalars( const char * buf,
      i = *start;
 
      while( i < max )

diff --git a/source/core_json.c b/source/core_json.c
@@ -971,14 +971,18 @@ static void skipArrayScalars( const char * buf,
  * @param[in,out] start  The index at which to begin.
  * @param[in] max  The size of the buffer.
  *
+ * @return true if a valid scalar key-value pairs were present;
+ * false otherwise.
+ *
  * @note Stops advance if a value is an object or array.
  */
-static void skipObjectScalars( const char * buf,
+static bool skipObjectScalars( const char * buf,
                                size_t * start,
                                size_t max )
 {
     size_t i = 0U;
     bool comma = false;
+    bool ret = true;
 
     coreJSON_ASSERT( ( buf != NULL ) && ( start != NULL ) && ( max > 0U ) );
 
@@ -988,13 +992,15 @@ static void skipObjectScalars( const char * buf,
     {
         if( skipString( buf, &i, max ) != true )
         {
+            ret = false;
             break;
         }
 
         skipSpace( buf, &i, max );
 
         if( ( i < max ) && ( buf[ i ] != ':' ) )
         {
+            ret = false;
             break;
         }
 
@@ -1009,6 +1015,7 @@ static void skipObjectScalars( const char * buf,
 
         if( skipAnyScalar( buf, &i, max ) != true )
         {
+            ret = false;
             break;
         }
 
@@ -1020,6 +1027,8 @@ static void skipObjectScalars( const char * buf,
             break;
         }
     }
+
+    return ret;
 }
 
 /**
@@ -1029,13 +1038,17 @@ static void skipObjectScalars( const char * buf,
  * @param[in,out] start  The index at which to begin.
  * @param[in] max  The size of the buffer.
  * @param[in] mode  The first character of an array '[' or object '{'.
+ *
+ * @return true if a valid scalers were present;
+ * false otherwise.
  */
-static void skipScalars( const char * buf,
+static bool skipScalars( const char * buf,
                          size_t * start,
                          size_t max,
                          char mode )
 {
     bool modeIsOpenBracket = ( bool ) isOpenBracket_( mode );
+    bool ret = true;
 
     /* assert function may be implemented in macro using a # or ## operator.
      * Using a local variable here to prevent macro replacement is subjected
@@ -1053,8 +1066,10 @@ static void skipScalars( const char * buf,
     }
     else
     {
-        skipObjectScalars( buf, start, max );
+        ret = skipObjectScalars( buf, start, max );
     }
+
+    return ret;
 }
 
 /**
@@ -1106,7 +1121,10 @@ static JSONStatus_t skipCollection( const char * buf,
                 }
 
                 stack[ depth ] = c;
-                skipScalars( buf, &i, max, stack[ depth ] );
+                if( skipScalars( buf, &i, max, stack[ depth ] ) != true )
+                {
+                    ret = JSONIllegalDocument;
+                }
                 break;
 
             case '}':
@@ -1120,7 +1138,10 @@ static JSONStatus_t skipCollection( const char * buf,
                     if( ( skipSpaceAndComma( buf, &i, max ) == true ) &&
                         isOpenBracket_( stack[ depth ] ) )
                     {
-                        skipScalars( buf, &i, max, stack[ depth ] );
+                        if( skipScalars( buf, &i, max, stack[ depth ] ) != true )
+                        {
+                            ret = JSONIllegalDocument;
+                        }
                     }
 
                     break;

diff --git a/test/cbmc/include/core_json_contracts.h b/test/cbmc/include/core_json_contracts.h
@@ -270,7 +270,7 @@ assigns( *start )
 ensures( isValidStart( *start, old( *start ), max ) )
 ;
 
-void skipObjectScalars( const char * buf,
+bool skipObjectScalars( const char * buf,
                         size_t * start,
                         size_t max )
 requires( isValidBufferWithStartIndex( buf, max, start ) )

diff --git a/test/unit-test/core_json_utest.c b/test/unit-test/core_json_utest.c
@@ -168,6 +168,15 @@
 #define MISSING_VALUE_AFTER_KEY                            "{\"foo\":{\"bar\":}}"
 #define MISSING_VALUE_AFTER_KEY_LENGTH                     ( sizeof( MISSING_VALUE_AFTER_KEY ) - 1 )
 
+#define MISSING_KEY                                        "{\"foo\":\"bar\", {\"abc\":\"xyz\"}}"
+#define MISSING_KEY_LENGTH                                 ( sizeof( MISSING_KEY ) - 1 )
+
+#define MISSING_VALUE                                      "{\"foo\":{\"abc\":\"xyz\"}, \"bar\":}"
+#define MISSING_VALUE_LENGTH                               ( sizeof( MISSING_VALUE ) - 1 )
+
+#define MISSING_SEPERATOR                                  "{\"foo\" \"bar\"}"
+#define MISSING_SEPERATOR_LENGTH                           ( sizeof( MISSING_SEPERATOR ) - 1 )
+
 #define MISMATCHED_BRACKETS                                "{\"foo\":{\"bar\":\"xyz\"]}"
 #define MISMATCHED_BRACKETS_LENGTH                         ( sizeof( MISMATCHED_BRACKETS ) - 1 )
 
@@ -619,6 +628,18 @@ void test_JSON_Validate_Illegal_Documents( void )
                                 MISSING_VALUE_AFTER_KEY_LENGTH );
     TEST_ASSERT_EQUAL( JSONIllegalDocument, jsonStatus );
 
+    jsonStatus = JSON_Validate( MISSING_KEY,
+                                MISSING_KEY_LENGTH );
+    TEST_ASSERT_EQUAL( JSONIllegalDocument, jsonStatus );
+
+    jsonStatus = JSON_Validate( MISSING_VALUE,
+                                MISSING_VALUE_LENGTH );
+    TEST_ASSERT_EQUAL( JSONIllegalDocument, jsonStatus );
+
+    jsonStatus = JSON_Validate( MISSING_SEPERATOR,
+                                MISSING_SEPERATOR_LENGTH );
+    TEST_ASSERT_EQUAL( JSONIllegalDocument, jsonStatus );
+
     jsonStatus = JSON_Validate( MISMATCHED_BRACKETS,
                                 MISMATCHED_BRACKETS_LENGTH );
     TEST_ASSERT_EQUAL( JSONIllegalDocument, jsonStatus );