-
Notifications
You must be signed in to change notification settings - Fork 74
Fix JSON validation issues concerning missing commas, escaped control characters, and hex escaped 0 value. #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
4c4f597
10fd66c
eebe8bc
e5c12ff
684af6d
92ab41f
94ed2de
8c5a830
ef1f39c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -321,8 +321,6 @@ static uint8_t hexToInt( char c ) | |
| * | ||
| * @return true if a valid escape sequence was present; | ||
| * false otherwise. | ||
| * | ||
| * @note For the sake of security, \u0000 is disallowed. | ||
| */ | ||
| static bool skipOneHexEscape( const char * buf, | ||
| size_t * start, | ||
|
|
@@ -358,7 +356,7 @@ static bool skipOneHexEscape( const char * buf, | |
| } | ||
| } | ||
|
|
||
| if( ( i == end ) && ( value > 0U ) ) | ||
| if( i == end ) | ||
| { | ||
| ret = true; | ||
| *outValue = value; | ||
|
|
@@ -382,8 +380,6 @@ static bool skipOneHexEscape( const char * buf, | |
| * | ||
| * @return true if a valid escape sequence was present; | ||
| * false otherwise. | ||
| * | ||
| * @note For the sake of security, \u0000 is disallowed. | ||
| */ | ||
| #define isHighSurrogate( x ) ( ( ( x ) >= 0xD800U ) && ( ( x ) <= 0xDBFFU ) ) | ||
| #define isLowSurrogate( x ) ( ( ( x ) >= 0xDC00U ) && ( ( x ) <= 0xDFFFU ) ) | ||
|
|
@@ -437,8 +433,6 @@ static bool skipHexEscape( const char * buf, | |
| * | ||
| * @return true if a valid escape sequence was present; | ||
| * false otherwise. | ||
| * | ||
| * @note For the sake of security, \NUL is disallowed. | ||
| */ | ||
| static bool skipEscape( const char * buf, | ||
| size_t * start, | ||
|
|
@@ -457,9 +451,6 @@ static bool skipEscape( const char * buf, | |
|
|
||
| switch( c ) | ||
| { | ||
| case '\0': | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will break from the default case - no need for this. |
||
| break; | ||
|
|
||
| case 'u': | ||
| ret = skipHexEscape( buf, &i, max ); | ||
| break; | ||
|
|
@@ -477,14 +468,6 @@ static bool skipEscape( const char * buf, | |
| break; | ||
|
|
||
| default: | ||
|
|
||
| /* a control character: (NUL,SPACE) */ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is invalid... escaped literal control characters should fail validation per spec, thus I have removed it. |
||
| if( iscntrl_( c ) ) | ||
| { | ||
| i += 2U; | ||
| ret = true; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } | ||
|
|
@@ -934,11 +917,12 @@ static bool skipSpaceAndComma( const char * buf, | |
| * | ||
| * @note Stops advance if a value is an object or array. | ||
| */ | ||
| static void skipArrayScalars( const char * buf, | ||
| static bool skipArrayScalars( const char * buf, | ||
| size_t * start, | ||
| size_t max ) | ||
| { | ||
| size_t i = 0U; | ||
| bool ret = true; | ||
|
|
||
| coreJSON_ASSERT( ( buf != NULL ) && ( start != NULL ) && ( max > 0U ) ); | ||
|
|
||
|
|
@@ -953,11 +937,19 @@ static void skipArrayScalars( const char * buf, | |
|
|
||
| if( skipSpaceAndComma( buf, &i, max ) != true ) | ||
| { | ||
| /* After parsing a scalar, we must either have a comma (followed by more content) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes cases such as [3[4]] from being parsed as valid. |
||
| * or be at a closing bracket. If neither, the array is malformed. */ | ||
| if( ( i >= max ) || !isSquareClose_( buf[ i ] ) ) | ||
| { | ||
| ret = false; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } | ||
|
|
||
| *start = i; | ||
| return ret; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1071,7 +1063,7 @@ static bool skipScalars( const char * buf, | |
| { | ||
| if( !isSquareClose_( buf[ i ] ) ) | ||
| { | ||
| skipArrayScalars( buf, start, max ); | ||
| ret = skipArrayScalars( buf, start, max ); | ||
| } | ||
| } | ||
| else | ||
|
|
@@ -1151,14 +1143,22 @@ static JSONStatus_t skipCollection( const char * buf, | |
| { | ||
| depth--; | ||
|
|
||
| if( ( skipSpaceAndComma( buf, &i, max ) == true ) && | ||
| isOpenBracket_( stack[ depth ] ) ) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have removed this isOpenBracket_ check as there is no point... stack[ depth ] will always contain an open bracket when depth >= 0. |
||
| if( skipSpaceAndComma( buf, &i, max ) == true ) | ||
| { | ||
| if( skipScalars( buf, &i, max, stack[ depth ] ) != true ) | ||
| { | ||
| ret = JSONIllegalDocument; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| /* After closing a nested collection, if there is no comma found when calling | ||
| * skipSpaceAndComma, then we must be at the end of the parent collection. */ | ||
| if( ( i < max ) && !isMatchingBracket_( stack[ depth ], buf[ i ] ) ) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes cases where missing commas between collections was considered valid. (e.g. [ { "key1" : "value1"} {"key2":"value2"}]) |
||
| { | ||
| ret = JSONIllegalDocument; | ||
| } | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow value of 0 (\u0000) per JSON spec.