Add pq-crypto/ to cppcheck (#2351)

* Add pq-crypto/ to cppcheck
* Convert line endings dos2unix on sike_r2/ec_isogeny.c
* Remove specific line numbers from cppcheck_suppressions.txt;
move suppressions inline where appropriate
* Fix indents
* More indent fixes
* More pq cppcheck fixes
* Remove unecessary cppcheck suppresions
* Really fix the indents this time

Author: bbutch

codebuild/bin/cppcheck_suppressions.txt

@@ -1,14 +1,38 @@
-// Message: [tests/unit/s2n_stuffer_text_test.c:28]: (style:variableScope) The scope of the variable 'text' can be reduced.
+// Message: (style:variableScope) The scope of the variable 'text' can be reduced.
 // Reason: Don't error for being able to reduce scope of variables in tests
 variableScope:tests/unit/*
 
-// cppcheck Message: [utils/s2n_socket.c:87]: (information:ConfigurationNotChecked) Skipping configuration 'SO_RCVLOWAT' since the value of 'SO_RCVLOWAT' is unknown. Use -D if you want to check it. You can use -U to skip it explicitly.
+// cppcheck Message: (information:ConfigurationNotChecked) Skipping configuration 'SO_RCVLOWAT' since the value of 'SO_RCVLOWAT' is unknown. Use -D if you want to check it. You can use -U to skip it explicitly.
 // Reason: There are many Config options that aren't checked by Cppcheck, and it warns for each. Ignore these so that they don't clutter the output.
 ConfigurationNotChecked:bin/s2nd.c
 ConfigurationNotChecked:tls/s2n_x509_validator.c
 ConfigurationNotChecked:utils/s2n_safety.c
 ConfigurationNotChecked:utils/s2n_socket.c
 
-// cppcheck Message: [tests/unit/s2n_timer_test.c:49]: (style:redundantAssignment) Variable 'mock_time' is reassigned a value before the old one has been used.
+// cppcheck Message: (information:ConfigurationNotChecked) Skipping configuration 'static_assert' since the value of 'static_assert' is unknown. Use -D if you want to check it. You can use -U to skip it explicitly.
+// Reason: Same as above.
+ConfigurationNotChecked:pq-crypto/bike_r1/bike_defs.h
+ConfigurationNotChecked:pq-crypto/bike_r1/gf2x_mul.c
+ConfigurationNotChecked:pq-crypto/bike_r1/sha.h
+ConfigurationNotChecked:pq-crypto/bike_r1/aes_ctr_prf.c
+ConfigurationNotChecked:pq-crypto/bike_r1/secure_decode_portable.c
+ConfigurationNotChecked:pq-crypto/bike_r1/decode.c
+ConfigurationNotChecked:pq-crypto/bike_r2/bike_defs.h
+ConfigurationNotChecked:pq-crypto/bike_r2/gf2x_mul.c
+ConfigurationNotChecked:pq-crypto/bike_r2/sha.h
+ConfigurationNotChecked:pq-crypto/bike_r2/aes_ctr_prf.c
+ConfigurationNotChecked:pq-crypto/bike_r2/secure_decode_portable.c
+ConfigurationNotChecked:pq-crypto/bike_r2/decode.c
+ConfigurationNotChecked:pq-crypto/bike_r2/bike_r2_kem.c
+
+// cppcheck Message: (style:redundantAssignment) Variable 'mock_time' is reassigned a value before the old one has been used.
 // Reason: s2n_config_set_monotonic_clock() takes a reference to mock_time so that whenever it's modified locally, the timer sees the update when it dereferences the pointer.
 redundantAssignment:tests/unit/s2n_timer_test.c
+
+// cppcheck Message: (error:shiftTooManyBits) Shifting 32-bit value by 64 bits is undefined behaviour.
+// Reason: In the SIKE arithmetic macros, this is actually a 128-bit typedef being shifted by 64 bits.
+shiftTooManyBits:pq-crypto/sike_r2/fp.c
+
+// cppcheck Message (information:unmatchedSuppression) Unmatched suppression: shiftTooManyBits
+// Reason: After applying the shiftTooManyBits suppression above, cppcheck complains (erroneously) that it is not necessary
+unmatchedSuppression:pq-crypto/sike_r2/fp.c

codebuild/bin/run_cppcheck.sh

@@ -30,8 +30,7 @@ CPPCHECK_EXECUTABLE=${CPPCHECK_INSTALL_DIR}/cppcheck/cppcheck
 
 FAILED=0
 $CPPCHECK_EXECUTABLE --version
-$CPPCHECK_EXECUTABLE --std=c99 --error-exitcode=-1 --quiet --force -j 8 --enable=all --template='[{file}:{line}]: ({severity}:{id}) {message}' --inline-suppr --suppressions-list=codebuild/bin/cppcheck_suppressions.txt -I . -I ./tests api bin crypto error stuffer ./tests/unit tls utils || FAILED=1
-
+$CPPCHECK_EXECUTABLE --std=c99 --error-exitcode=-1 --quiet --force -j 8 --enable=all --template='[{file}:{line}]: ({severity}:{id}) {message}' --inline-suppr --suppressions-list=codebuild/bin/cppcheck_suppressions.txt -I . -I ./tests api bin crypto error stuffer ./tests/unit tls utils pq-crypto || FAILED=1
 if [ $FAILED == 1 ];
 then
 	printf "\\033[31;1mFAILED cppcheck\\033[0m\\n"

pq-crypto/bike_r1/aes_wrap.h

@@ -66,5 +66,5 @@ _INLINE_ void
 aes256_free_ks(OUT aes256_ks_t *ks)
 {
   EVP_CIPHER_CTX_free(*ks);
-  ks = NULL;
+  *ks = NULL;
 }

pq-crypto/bike_r1/gf2x_portable.c

@@ -19,7 +19,7 @@
 void
 gf2x_mul_1x1(uint64_t *c, uint64_t a, uint64_t b)
 {
-  uint64_t       h = 0, l = 0, g1, g2, u[8];
+  uint64_t       h = 0, l = 0, u[8];
   const uint64_t w = 64;
   const uint64_t s = 3;
   // Multiplying 64 bits by 7 can results in an overflow of 3 bits.
@@ -41,8 +41,8 @@ gf2x_mul_1x1(uint64_t *c, uint64_t a, uint64_t b)
   h = (u[(a >> 3) & 7] >> 61);
   for(uint32_t i = (2 * s); i < w; i += (2 * s))
   {
-    g1 = u[(a >> i) & 7];
-    g2 = u[(a >> (i + s)) & 7];
+    uint64_t g1 = u[(a >> i) & 7];
+    uint64_t g2 = u[(a >> (i + s)) & 7];
 
     l ^= (g1 << i) ^ (g2 << (i + s));
     h ^= (g1 >> (w - i)) ^ (g2 >> (w - (i + s)));

pq-crypto/bike_r1/openssl_utils.c

@@ -34,11 +34,10 @@ _INLINE_ void
 reverse_endian(OUT uint8_t *res, IN const uint8_t *in, IN const uint32_t n)
 {
   uint32_t i;
-  uint64_t tmp;
 
   for(i = 0; i < (n / 2); i++)
   {
-    tmp            = in[i];
+    uint64_t tmp            = in[i];
     res[i]         = in[n - 1 - i];
     res[n - 1 - i] = tmp;
   }

pq-crypto/bike_r1/sampling.c

@@ -31,7 +31,7 @@ get_rand_mod_len(OUT uint32_t *    rand_pos,
       break;
     }
 
-  } while(1 == 1);
+  } while(1);
 
   return SUCCESS;
 }

pq-crypto/bike_r1/sampling_portable.c

@@ -24,7 +24,6 @@ secure_set_bits(IN OUT uint64_t * a,
   assert(weight <= MAX_WEIGHT);
   uint64_t qw_pos[MAX_WEIGHT];
   uint64_t bit_pos[MAX_WEIGHT];
-  uint64_t tmp = 0;
 
   // 1. Identify the QW position of each value and the bit position inside this
   // QW.
@@ -37,7 +36,7 @@ secure_set_bits(IN OUT uint64_t * a,
   // 2. Fill each QW in a constant time.
   for(uint32_t qw = 0; qw < (a_len_bytes / 8); qw++)
   {
-    tmp = 0;
+    uint64_t tmp = 0;
     for(uint32_t j = 0; j < weight; j++)
     {
       uint64_t mask = (-1ULL) + (!secure_cmp32(qw_pos[j], qw));

pq-crypto/bike_r2/aes_wrap.h

@@ -67,5 +67,5 @@ _INLINE_ void
 aes256_free_ks(OUT aes256_ks_t *ks)
 {
   EVP_CIPHER_CTX_free(*ks);
-  ks = NULL;
+  *ks = NULL;
 }

pq-crypto/bike_r2/gf2x_portable.c

@@ -19,7 +19,7 @@
 void
 gf2x_mul_1x1(uint64_t *c, uint64_t a, uint64_t b)
 {
-  uint64_t       h = 0, l = 0, g1, g2, u[8];
+  uint64_t       h = 0, l = 0, u[8];
   const uint64_t w = 64;
   const uint64_t s = 3;
   // Multiplying 64 bits by 7 can results in an overflow of 3 bits.
@@ -41,8 +41,8 @@ gf2x_mul_1x1(uint64_t *c, uint64_t a, uint64_t b)
   h = (u[(a >> 3) & 7] >> 61);
   for(uint32_t i = (2 * s); i < w; i += (2 * s))
   {
-    g1 = u[(a >> i) & 7];
-    g2 = u[(a >> (i + s)) & 7];
+    uint64_t g1 = u[(a >> i) & 7];
+    uint64_t g2 = u[(a >> (i + s)) & 7];
 
     l ^= (g1 << i) ^ (g2 << (i + s));
     h ^= (g1 >> (w - i)) ^ (g2 >> (w - (i + s)));

pq-crypto/bike_r2/openssl_utils.c

@@ -34,11 +34,10 @@ _INLINE_ void
 reverse_endian(OUT uint8_t *res, IN const uint8_t *in, IN const uint32_t n)
 {
   uint32_t i;
-  uint64_t tmp;
 
   for(i = 0; i < (n / 2); i++)
   {
-    tmp            = in[i];
+    uint64_t tmp            = in[i];
     res[i]         = in[n - 1 - i];
     res[n - 1 - i] = tmp;
   }