From a1c6b7045ce012c1ad746247c17fa4c07e619a1c Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Tue, 12 Jul 2022 21:39:29 +0000 Subject: [PATCH 01/14] Updated the MISRA.md and misra.config files after meeting with senior SDE. Put inline supression for a comparison related to the SIZE_MAX macro. Want to get clarification about the line before putting a change in. --- MISRA.md | 5 ++++- source/core_json.c | 1 + tools/coverity/misra.config | 31 +++++++++++++++++++++++++------ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/MISRA.md b/MISRA.md index 15a6749d..61dc40eb 100644 --- a/MISRA.md +++ b/MISRA.md @@ -8,9 +8,13 @@ Deviations from the MISRA standard are listed below: | Deviation | Category | Justification | | :-: | :-: | :-: | | Directive 4.9 | Advisory | Allow inclusion of function like macros. | +| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | | Rule 3.1 | Required | Allow nested comments. C++ style `//` comments are used in example code within Doxygen documentation blocks. | +| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | | Rule 8.13 | Advisory | Allow one function to have a char * argument without const qualifier. | +| Rule 12.3 | Advisory | Allow use of `assert()` macro, expansion of which uses comma operator. | | Rule 15.4 | Advisory | Allow more then one `break` statement to terminate a loop. | +| Rule 15.6 | Required | Allow use of `assert()` macro, expansion of which contains non-compound if statements. | | Rule 19.2 | Advisory | Allow a `union` of a signed and unsigned type of identical sizes. | | Rule 20.12 | Required | Allow use of `assert()`, which uses a parameter in both expanded and raw forms. | @@ -18,7 +22,6 @@ Deviations from the MISRA standard are listed below: | Deviation | Category | Justification | | :-: | :-: | :-: | | Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | -| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | ### Suppressed with Coverity Comments | Deviation | Category | Justification | diff --git a/source/core_json.c b/source/core_json.c index bf764c9f..57069e82 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -335,6 +335,7 @@ static bool skipOneHexEscape( const char * buf, i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ + /* coverity[misra_c_2012_rule_14_3_violation] */ end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 8f310375..10a6d130 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -10,30 +10,49 @@ category: "Advisory", reason: "Allow inclusion of function like macros." }, + { + deviation: "Rule 2.5", + reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file." + }, + { + deviation: "Rule 3.1", + category: "Required", + reason: "Allow nested comments. Documentation blocks contain comments for example code." + }, + { + deviation: "Rule 8.7", + reason: "API functions are not used by library. They must be externally visible in order to be used by the application." + }, { deviation: "Rule 8.13", category: "Advisory", reason: "Allow one function to have a char * argument without const qualifier." }, + { + deviation: "Rule 12.3", + category: "Advisory", + reason: "Allow use of assert(), expansion of which uses comma operator." + }, { deviation: "Rule 15.4", category: "Advisory", reason: "Allow more then one break statement to terminate a loop" }, + { + deviation: "Rule 15.6", + category: "Required", + reason: "Allow use of assert(), expansion of which contains non-compound if statements." + }, + { deviation: "Rule 19.2", category: "Advisory", reason: "Allow a union of a signed and unsigned type of identical sizes." }, - { - deviation: "Rule 3.1", - category: "Required", - reason: "Allow nested comments. Documentation blocks contain comments for example code." - }, { deviation: "Rule 20.12", category: "Required", reason: "Allow use of assert(), which uses a parameter in both expanded and raw forms." - }, + } ] } From f4011a772fbbffd87b1556f016ef57cf0da2848d Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 14 Jul 2022 01:18:02 +0000 Subject: [PATCH 02/14] Fixing some whitespace/formatting issues --- MISRA.md | 1 - tools/coverity/misra.config | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/MISRA.md b/MISRA.md index 61dc40eb..88227b7c 100644 --- a/MISRA.md +++ b/MISRA.md @@ -21,7 +21,6 @@ Deviations from the MISRA standard are listed below: ### Flagged by Coverity | Deviation | Category | Justification | | :-: | :-: | :-: | -| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | ### Suppressed with Coverity Comments | Deviation | Category | Justification | diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 10a6d130..55ea7dee 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -43,7 +43,6 @@ category: "Required", reason: "Allow use of assert(), expansion of which contains non-compound if statements." }, - { deviation: "Rule 19.2", category: "Advisory", @@ -53,6 +52,6 @@ deviation: "Rule 20.12", category: "Required", reason: "Allow use of assert(), which uses a parameter in both expanded and raw forms." - } + }, ] } From 9429802b23c2dfcf83720b0d9b2713359c327650 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 27 Jul 2022 18:49:29 +0000 Subject: [PATCH 03/14] Changing MISRA.md file to reflect new format, modified inline supression in source file to match new formatting --- MISRA.md | 40 +++++++++++++++++++--------------------- source/core_json.c | 7 ++++--- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/MISRA.md b/MISRA.md index 88227b7c..4ebbde35 100644 --- a/MISRA.md +++ b/MISRA.md @@ -2,27 +2,25 @@ The coreJSON library files conform to the [MISRA C:2012](https://www.misra.org.uk) guidelines, with some noted exceptions. Compliance is checked with Coverity static analysis. -Deviations from the MISRA standard are listed below: +The specific deviations, suppressed inline, are listed below. -### Ignored by [Coverity Configuration](tools/coverity/misra.config) -| Deviation | Category | Justification | -| :-: | :-: | :-: | -| Directive 4.9 | Advisory | Allow inclusion of function like macros. | -| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | -| Rule 3.1 | Required | Allow nested comments. C++ style `//` comments are used in example code within Doxygen documentation blocks. | -| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | -| Rule 8.13 | Advisory | Allow one function to have a char * argument without const qualifier. | -| Rule 12.3 | Advisory | Allow use of `assert()` macro, expansion of which uses comma operator. | -| Rule 15.4 | Advisory | Allow more then one `break` statement to terminate a loop. | -| Rule 15.6 | Required | Allow use of `assert()` macro, expansion of which contains non-compound if statements. | -| Rule 19.2 | Advisory | Allow a `union` of a signed and unsigned type of identical sizes. | -| Rule 20.12 | Required | Allow use of `assert()`, which uses a parameter in both expanded and raw forms. | - -### Flagged by Coverity -| Deviation | Category | Justification | -| :-: | :-: | :-: | +Additionally, [MISRA configuration file](https://github.com/FreeRTOS/coreJSON/blob/main/tools/coverity/misra.config) contains the project wide deviations. ### Suppressed with Coverity Comments -| Deviation | Category | Justification | -| :-: | :-: | :-: | -| Rule 11.3 | Required | False positive - the rule permits type qualifiers to be added. | +To find the violation references in the source files run grep on the source code +with ( Assuming rule 11.4 violation; with justification in point 2 ): +``` +grep 'MISRA Ref 11.4.2' . -rI + +#### Rule 11.3 +_Ref 11.3.1_ + +- MISRA C-2012 Rule 11.3 prohibits casting a pointer to a different type. + This instance is a false positive, as the rule permits the + addition of a type qualifier. + +#### Rule 14.3 +_Ref 14.3.1_ + +- MISRA C-2012 Rule 14.3 False positive as the value might be changed + depending on the conditionally compiled code diff --git a/source/core_json.c b/source/core_json.c index 57069e82..996df8a6 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -335,6 +335,8 @@ static bool skipOneHexEscape( const char * buf, i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ + /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */ /* coverity[misra_c_2012_rule_14_3_violation] */ end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; @@ -1678,9 +1680,8 @@ JSONStatus_t JSON_SearchT( char * buf, size_t * outValueLength, JSONTypes_t * outType ) { - /* MISRA Rule 11.3 prohibits casting a pointer to a different type. - * This instance is a false positive, as the rule permits the - * addition of a type qualifier. */ + /* MISRA Ref 11.3.1 [[Misaligned access]] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */ /* coverity[misra_c_2012_rule_11_3_violation] */ return JSON_SearchConst( ( const char * ) buf, max, query, queryLength, ( const char ** ) outValue, outValueLength, outType ); From a9b884018f7384e503e0291f0c602bc187254e2a Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 27 Jul 2022 18:55:37 +0000 Subject: [PATCH 04/14] Adding words to lexicon, and fixing links --- lexicon.txt | 2 ++ source/core_json.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lexicon.txt b/lexicon.txt index d4b0972b..185a9b0b 100644 --- a/lexicon.txt +++ b/lexicon.txt @@ -32,8 +32,10 @@ fd fe ff ffff +freertos foo gcc +github html https ifndef diff --git a/source/core_json.c b/source/core_json.c index 996df8a6..ba6cd23b 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -336,7 +336,7 @@ static bool skipOneHexEscape( const char * buf, i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */ + /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-143 */ /* coverity[misra_c_2012_rule_14_3_violation] */ end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; @@ -1681,7 +1681,7 @@ JSONStatus_t JSON_SearchT( char * buf, JSONTypes_t * outType ) { /* MISRA Ref 11.3.1 [[Misaligned access]] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-143 */ + /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-113 */ /* coverity[misra_c_2012_rule_11_3_violation] */ return JSON_SearchConst( ( const char * ) buf, max, query, queryLength, ( const char ** ) outValue, outValueLength, outType ); From 7fe33fc9c2670b39938fa0215e8f44c6391df7d5 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 27 Jul 2022 21:26:00 +0000 Subject: [PATCH 05/14] Minor update to MISRA.md file to use an actual violation as the example, and expanding on a message --- MISRA.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/MISRA.md b/MISRA.md index 4ebbde35..d66c59b6 100644 --- a/MISRA.md +++ b/MISRA.md @@ -8,16 +8,17 @@ Additionally, [MISRA configuration file](https://github.com/FreeRTOS/coreJSON/bl ### Suppressed with Coverity Comments To find the violation references in the source files run grep on the source code -with ( Assuming rule 11.4 violation; with justification in point 2 ): +with ( Assuming rule 14.3 violation; with justification in point 1 ): +``` +grep 'MISRA Ref 14.3.1' . -rI ``` -grep 'MISRA Ref 11.4.2' . -rI #### Rule 11.3 _Ref 11.3.1_ - MISRA C-2012 Rule 11.3 prohibits casting a pointer to a different type. This instance is a false positive, as the rule permits the - addition of a type qualifier. + addition of a const type qualifier. #### Rule 14.3 _Ref 14.3.1_ From 908a141b328becbe83e52c31c8e4ff47f8af5eb6 Mon Sep 17 00:00:00 2001 From: Skptak Date: Wed, 3 Aug 2022 12:37:48 -0700 Subject: [PATCH 06/14] Update source/core_json.c Remove extra set of square brackets Co-authored-by: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> --- source/core_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core_json.c b/source/core_json.c index ba6cd23b..2d41dc5b 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -1680,7 +1680,7 @@ JSONStatus_t JSON_SearchT( char * buf, size_t * outValueLength, JSONTypes_t * outType ) { - /* MISRA Ref 11.3.1 [[Misaligned access]] */ + /* MISRA Ref 11.3.1 [Misaligned access] */ /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-113 */ /* coverity[misra_c_2012_rule_11_3_violation] */ return JSON_SearchConst( ( const char * ) buf, max, query, queryLength, From 8957a186922d1b3d1f39ab3ca34d92809b8c5659 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 11 Aug 2022 22:07:28 +0000 Subject: [PATCH 07/14] Changes to the way we determine the end in skipOneHexEscape() --- MISRA.md | 10 ++-------- source/core_json.c | 12 ++++++++---- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/MISRA.md b/MISRA.md index d66c59b6..825dea7d 100644 --- a/MISRA.md +++ b/MISRA.md @@ -8,9 +8,9 @@ Additionally, [MISRA configuration file](https://github.com/FreeRTOS/coreJSON/bl ### Suppressed with Coverity Comments To find the violation references in the source files run grep on the source code -with ( Assuming rule 14.3 violation; with justification in point 1 ): +with ( Assuming rule 11.3 violation; with justification in point 1 ): ``` -grep 'MISRA Ref 14.3.1' . -rI +grep 'MISRA Ref 11.3.1' . -rI ``` #### Rule 11.3 @@ -19,9 +19,3 @@ _Ref 11.3.1_ - MISRA C-2012 Rule 11.3 prohibits casting a pointer to a different type. This instance is a false positive, as the rule permits the addition of a const type qualifier. - -#### Rule 14.3 -_Ref 14.3.1_ - -- MISRA C-2012 Rule 14.3 False positive as the value might be changed - depending on the conditionally compiled code diff --git a/source/core_json.c b/source/core_json.c index 2d41dc5b..2d5100fc 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -335,10 +335,14 @@ static bool skipOneHexEscape( const char * buf, i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ - /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ - /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-143 */ - /* coverity[misra_c_2012_rule_14_3_violation] */ - end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; + if ( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) + { + end = ( i + HEX_ESCAPE_LENGTH ); + } + else + { + end = max; + } if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) { From e61d81bf7c6710beb16035f0c7e2f22cba8d2bb4 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 11 Aug 2022 22:20:10 +0000 Subject: [PATCH 08/14] Removed a redundant check of a variable --- source/core_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core_json.c b/source/core_json.c index 2d5100fc..bb38a945 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -344,7 +344,7 @@ static bool skipOneHexEscape( const char * buf, end = max; } - if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) + if( ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) { for( i += 2U; i < end; i++ ) { From 1207a6768a8b421dd91b77784c9c5b263db0b0a8 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 11 Aug 2022 23:05:00 +0000 Subject: [PATCH 09/14] Formatting fix and adding a test in to get more line coverage --- source/core_json.c | 7 ++++--- test/unit-test/core_json_utest.c | 3 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/source/core_json.c b/source/core_json.c index bb38a945..49824659 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -335,13 +335,14 @@ static bool skipOneHexEscape( const char * buf, i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ - if ( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) + + if( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) { - end = ( i + HEX_ESCAPE_LENGTH ); + end = ( i + HEX_ESCAPE_LENGTH ); } else { - end = max; + end = max; } if( ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) diff --git a/test/unit-test/core_json_utest.c b/test/unit-test/core_json_utest.c index 8f93a148..f2f90b40 100644 --- a/test/unit-test/core_json_utest.c +++ b/test/unit-test/core_json_utest.c @@ -1897,4 +1897,7 @@ void test_JSON_overflows( void ) start = SIZE_MAX; TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, SIZE_MAX, &u ) ); + TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, -1, &u ) ); + start = 5; + TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, 15u, &u ) ); } From d542e7967eaa7034c1a0b6e1fb5e878adf60d035 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 11 Aug 2022 23:09:57 +0000 Subject: [PATCH 10/14] Trying to reach 100% branch coverage --- test/unit-test/core_json_utest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit-test/core_json_utest.c b/test/unit-test/core_json_utest.c index f2f90b40..3c7fb815 100644 --- a/test/unit-test/core_json_utest.c +++ b/test/unit-test/core_json_utest.c @@ -1898,6 +1898,5 @@ void test_JSON_overflows( void ) start = SIZE_MAX; TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, SIZE_MAX, &u ) ); TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, -1, &u ) ); - start = 5; - TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, 15u, &u ) ); + TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, 5u, &u ) ); } From 7b23cc72b9a0acc1847e4b38fee9a9a61b2d6fbc Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 11 Aug 2022 23:17:17 +0000 Subject: [PATCH 11/14] Adding the removal of debug for coverity target, and then removing two rule exceptions from the misra.config due to the change --- test/CMakeLists.txt | 3 +++ tools/coverity/misra.config | 10 ---------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0af41e3f..80df584a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -41,6 +41,9 @@ add_library( coverity_analysis # JSON public include path. target_include_directories( coverity_analysis PUBLIC ${JSON_INCLUDE_PUBLIC_DIRS} ) +# When building the coverity analysis target we disable debug +target_compile_options(coverity_analysis PUBLIC -DNDEBUG ) + # ==================================== Test Configuration ======================================== # Include Unity build configuration. diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 55ea7dee..2bbb3396 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -28,21 +28,11 @@ category: "Advisory", reason: "Allow one function to have a char * argument without const qualifier." }, - { - deviation: "Rule 12.3", - category: "Advisory", - reason: "Allow use of assert(), expansion of which uses comma operator." - }, { deviation: "Rule 15.4", category: "Advisory", reason: "Allow more then one break statement to terminate a loop" }, - { - deviation: "Rule 15.6", - category: "Required", - reason: "Allow use of assert(), expansion of which contains non-compound if statements." - }, { deviation: "Rule 19.2", category: "Advisory", From 910b995c4de677ba1dd3aa1ba8cc5a8014f81c93 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 15 Aug 2022 15:03:15 +0000 Subject: [PATCH 12/14] skipOneHexEscape had a line that was flagged as a MISRA 14.3 rule violation. It was flagged because it believed that the if statement comparison was invariant. This could be proven as a bug by assigning the variable a value larger than the comparison, and then still receiving the violation. A logic change has been made to get around this, but it now fails CBMC proofs. Trying a different if statement to see if this passes checks. --- source/core_json.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/core_json.c b/source/core_json.c index 49824659..9046317f 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -333,7 +333,6 @@ static bool skipOneHexEscape( const char * buf, assert( ( buf != NULL ) && ( start != NULL ) && ( max > 0U ) ); assert( outValue != NULL ); - i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ if( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) @@ -345,7 +344,7 @@ static bool skipOneHexEscape( const char * buf, end = max; } - if( ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) + if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) { for( i += 2U; i < end; i++ ) { From 06b62809f456bd062a51fed1215e9ccc13582f3e Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 15 Aug 2022 15:08:15 +0000 Subject: [PATCH 13/14] Forgot to add inital assign back in --- source/core_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core_json.c b/source/core_json.c index 9046317f..9e537498 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -332,7 +332,7 @@ static bool skipOneHexEscape( const char * buf, assert( ( buf != NULL ) && ( start != NULL ) && ( max > 0U ) ); assert( outValue != NULL ); - + i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ if( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) From c705d55d23781d49ae18465a1468a07f92545c9f Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 15 Aug 2022 15:57:42 +0000 Subject: [PATCH 14/14] After a lot of attempts to create a MISRA and CBMC compliant version of skipOneHexEscape() I believe proof was found that shows the MISRA violation is a false flag. Due to this I believe that this should simply receive a suppression and the focus should be on the CBMC proofs. --- MISRA.md | 10 ++++++++++ source/core_json.c | 13 +++++-------- test/unit-test/core_json_utest.c | 2 -- tools/coverity/misra.config | 5 ----- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/MISRA.md b/MISRA.md index 825dea7d..f186e515 100644 --- a/MISRA.md +++ b/MISRA.md @@ -19,3 +19,13 @@ _Ref 11.3.1_ - MISRA C-2012 Rule 11.3 prohibits casting a pointer to a different type. This instance is a false positive, as the rule permits the addition of a const type qualifier. + +#### Rule 14.3 +_Ref 14.3.1_ + +- MISRA C-2012 Rule 14.3 False positive as the static analysis tool believes + i can never be larger than SIZE_MAX - HEX_ESCAPE_LENGTH. This can be proven as + a bug by setting i to be 18446744073709551615UL at initial assignment, then require + start != NULL before assigning the vaue of i to start. This creates a case + where i should be large enough to hit the else statement, but the tool still flags + this as invariant. diff --git a/source/core_json.c b/source/core_json.c index 9e537498..c549a4f2 100644 --- a/source/core_json.c +++ b/source/core_json.c @@ -332,17 +332,14 @@ static bool skipOneHexEscape( const char * buf, assert( ( buf != NULL ) && ( start != NULL ) && ( max > 0U ) ); assert( outValue != NULL ); + i = *start; #define HEX_ESCAPE_LENGTH ( 6U ) /* e.g., \u1234 */ - if( ( max >= HEX_ESCAPE_LENGTH ) && ( i <= ( max - HEX_ESCAPE_LENGTH ) ) ) - { - end = ( i + HEX_ESCAPE_LENGTH ); - } - else - { - end = max; - } + /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ + /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-143 */ + /* coverity[misra_c_2012_rule_14_3_violation] */ + end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) ) { diff --git a/test/unit-test/core_json_utest.c b/test/unit-test/core_json_utest.c index 3c7fb815..8f93a148 100644 --- a/test/unit-test/core_json_utest.c +++ b/test/unit-test/core_json_utest.c @@ -1897,6 +1897,4 @@ void test_JSON_overflows( void ) start = SIZE_MAX; TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, SIZE_MAX, &u ) ); - TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, -1, &u ) ); - TEST_ASSERT_EQUAL( false, skipOneHexEscape( buf, &start, 5u, &u ) ); } diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 2bbb3396..c4f53306 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -38,10 +38,5 @@ category: "Advisory", reason: "Allow a union of a signed and unsigned type of identical sizes." }, - { - deviation: "Rule 20.12", - category: "Required", - reason: "Allow use of assert(), which uses a parameter in both expanded and raw forms." - }, ] }