Skip to content

Commit 7f53bcc

Browse files
douzzerdouzzer
committed
fixes for clang-tidy reported defects and misstylings --with-liboqs:
* readability-named-parameter (style) * bugprone-sizeof-expression (true bugs) * clang-analyzer-deadcode.DeadStores (true bugs) * clang-analyzer-core.NonNullParamChecker (true bug) * clang-diagnostic-newline-eof (style) * clang-diagnostic-shorten-64-to-32 (true but benign in practice) fixes for sanitizer reported defects --with-liboqs: null pointer memcpy()s in TLSX_KeyShare_GenPqcKey() and server_generate_pqc_ciphertext(). fixes for silent crypto-critical failure in wolfSSL_liboqsGetRandomData(): refactor to accommodate oversize numOfBytes, and abort() if wc_RNG_GenerateBlock() returns failure.
1 parent 9e468a9 commit 7f53bcc

11 files changed

Lines changed: 42 additions & 22 deletions

File tree

‎src/tls.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7722,7 +7722,8 @@ static int TLSX_KeyShare_GenPqcKey(WOLFSSL *ssl, KeyShareEntry* kse)
77227722
ret = wc_KyberKey_EncodePrivateKey(kem, privKey, privSz);
77237723
}
77247724
if (ret == 0) {
7725-
XMEMCPY(pubKey, ecc_kse->pubKey, ecc_kse->pubKeyLen);
7725+
if (ecc_kse->pubKeyLen > 0)
7726+
XMEMCPY(pubKey, ecc_kse->pubKey, ecc_kse->pubKeyLen);
77267727
kse->pubKey = pubKey;
77277728
kse->pubKeyLen = ecc_kse->pubKeyLen + pubSz;
77287729
pubKey = NULL;
@@ -9010,7 +9011,8 @@ static int server_generate_pqc_ciphertext(WOLFSSL* ssl,
90109011
keyShareEntry->keLen = outlen + ssSz;
90119012
sharedSecret = NULL;
90129013

9013-
XMEMCPY(ciphertext, ecc_kse->pubKey, ecc_kse->pubKeyLen);
9014+
if (ecc_kse->pubKeyLen > 0)
9015+
XMEMCPY(ciphertext, ecc_kse->pubKey, ecc_kse->pubKeyLen);
90149016
keyShareEntry->pubKey = ciphertext;
90159017
keyShareEntry->pubKeyLen = (word32)(ecc_kse->pubKeyLen + ctSz);
90169018
ciphertext = NULL;

‎wolfcrypt/benchmark/benchmark.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12939,7 +12939,9 @@ int wolfcrypt_benchmark_main(int argc, char** argv)
1293912939
/* Both bench_pq_asym_opt and bench_pq_asym_opt2 are looking for
1294012940
* -pq, so we need to do a special case for -pq since optMatched
1294112941
* was set to 1 just above. */
12942-
if (string_matches(argv[1], bench_pq_asym_opt[0].str)) {
12942+
if ((bench_pq_asym_opt[0].str != NULL) &&
12943+
string_matches(argv[1], bench_pq_asym_opt[0].str))
12944+
{
1294312945
bench_pq_asym_algs2 |= bench_pq_asym_opt2[0].val;
1294412946
bench_all = 0;
1294512947
optMatched = 1;

‎wolfcrypt/src/dilithium.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ int wc_dilithium_init(dilithium_key* key)
205205
return BAD_FUNC_ARG;
206206
}
207207

208-
ForceZero(key, sizeof(key));
208+
ForceZero(key, sizeof(*key));
209209
return 0;
210210
}
211211

@@ -258,7 +258,7 @@ int wc_dilithium_get_level(dilithium_key* key, byte* level)
258258
void wc_dilithium_free(dilithium_key* key)
259259
{
260260
if (key != NULL) {
261-
ForceZero(key, sizeof(key));
261+
ForceZero(key, sizeof(*key));
262262
}
263263
}
264264

‎wolfcrypt/src/falcon.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ int wc_falcon_init(falcon_key* key)
197197
return BAD_FUNC_ARG;
198198
}
199199

200-
ForceZero(key, sizeof(key));
200+
ForceZero(key, sizeof(*key));
201201
return 0;
202202
}
203203

@@ -250,7 +250,7 @@ int wc_falcon_get_level(falcon_key* key, byte* level)
250250
void wc_falcon_free(falcon_key* key)
251251
{
252252
if (key != NULL) {
253-
ForceZero(key, sizeof(key));
253+
ForceZero(key, sizeof(*key));
254254
}
255255
}
256256

‎wolfcrypt/src/port/liboqs/liboqs.c‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ implementations for Post-Quantum cryptography algorithms.
3333

3434
#include <wolfssl/wolfcrypt/settings.h>
3535
#include <wolfssl/wolfcrypt/types.h>
36+
#include <wolfssl/wolfcrypt/logging.h>
3637
#include <wolfssl/wolfcrypt/error-crypt.h>
3738

3839
#include <wolfssl/wolfcrypt/port/liboqs/liboqs.h>
@@ -50,9 +51,24 @@ static int liboqs_init = 0;
5051

5152
static void wolfSSL_liboqsGetRandomData(uint8_t* buffer, size_t numOfBytes)
5253
{
53-
int ret = wc_RNG_GenerateBlock(liboqsCurrentRNG, buffer, (word32)numOfBytes);
54-
if (ret != 0) {
55-
// ToDo: liboqs exits programm if RNG fails, not sure what to do here
54+
int ret;
55+
word32 numOfBytes_word32;
56+
57+
while (numOfBytes > 0) {
58+
numOfBytes_word32 = (word32)numOfBytes;
59+
numOfBytes -= numOfBytes_word32;
60+
ret = wc_RNG_GenerateBlock(liboqsCurrentRNG, buffer,
61+
numOfBytes_word32);
62+
if (ret != 0) {
63+
/* ToDo: liboqs exits programm if RNG fails,
64+
* not sure what to do here
65+
*/
66+
WOLFSSL_MSG_EX(
67+
"wc_RNG_GenerateBlock(..., %u) failed with ret %d "
68+
"in wolfSSL_liboqsGetRandomData().", numOfBytes_word32, ret
69+
);
70+
abort();
71+
}
5672
}
5773
}
5874

‎wolfcrypt/src/sphincs.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ int wc_sphincs_init(sphincs_key* key)
243243
return BAD_FUNC_ARG;
244244
}
245245

246-
ForceZero(key, sizeof(key));
246+
ForceZero(key, sizeof(*key));
247247
return 0;
248248
}
249249

@@ -308,7 +308,7 @@ int wc_sphincs_get_level_and_optim(sphincs_key* key, byte* level, byte* optim)
308308
void wc_sphincs_free(sphincs_key* key)
309309
{
310310
if (key != NULL) {
311-
ForceZero(key, sizeof(key));
311+
ForceZero(key, sizeof(*key));
312312
}
313313
}
314314

@@ -857,7 +857,7 @@ int wc_Sphincs_PrivateKeyDecode(const byte* input, word32* inOutIdx,
857857
else if ((key->level == 5) && (key->optim == FAST_VARIANT)) {
858858
keytype = SPHINCS_FAST_LEVEL5k;
859859
}
860-
if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
860+
else if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
861861
keytype = SPHINCS_SMALL_LEVEL1k;
862862
}
863863
else if ((key->level == 3) && (key->optim == SMALL_VARIANT)) {
@@ -905,7 +905,7 @@ int wc_Sphincs_PublicKeyDecode(const byte* input, word32* inOutIdx,
905905
else if ((key->level == 5) && (key->optim == FAST_VARIANT)) {
906906
keytype = SPHINCS_FAST_LEVEL5k;
907907
}
908-
if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
908+
else if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
909909
keytype = SPHINCS_SMALL_LEVEL1k;
910910
}
911911
else if ((key->level == 3) && (key->optim == SMALL_VARIANT)) {
@@ -960,7 +960,7 @@ int wc_Sphincs_PublicKeyToDer(sphincs_key* key, byte* output, word32 inLen,
960960
else if ((key->level == 5) && (key->optim == FAST_VARIANT)) {
961961
keytype = SPHINCS_FAST_LEVEL5k;
962962
}
963-
if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
963+
else if ((key->level == 1) && (key->optim == SMALL_VARIANT)) {
964964
keytype = SPHINCS_SMALL_LEVEL1k;
965965
}
966966
else if ((key->level == 3) && (key->optim == SMALL_VARIANT)) {

‎wolfssl/wolfcrypt/dilithium.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ int wc_dilithium_import_private_key(const byte* priv, word32 privSz,
110110
dilithium_key* key);
111111

112112
WOLFSSL_API
113-
int wc_dilithium_export_public(dilithium_key*, byte* out, word32* outLen);
113+
int wc_dilithium_export_public(dilithium_key* key, byte* out, word32* outLen);
114114
WOLFSSL_API
115115
int wc_dilithium_export_private_only(dilithium_key* key, byte* out, word32* outLen);
116116
WOLFSSL_API

‎wolfssl/wolfcrypt/falcon.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ int wc_falcon_import_private_key(const byte* priv, word32 privSz,
105105
falcon_key* key);
106106

107107
WOLFSSL_API
108-
int wc_falcon_export_public(falcon_key*, byte* out, word32* outLen);
108+
int wc_falcon_export_public(falcon_key* key, byte* out, word32* outLen);
109109
WOLFSSL_API
110110
int wc_falcon_export_private_only(falcon_key* key, byte* out, word32* outLen);
111111
WOLFSSL_API

‎wolfssl/wolfcrypt/kyber.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@
4949

5050

5151
/* Size of a polynomial vector based on dimensions. */
52-
#define KYBER_POLY_VEC_SZ(k) (k * KYBER_POLY_SIZE)
52+
#define KYBER_POLY_VEC_SZ(k) ((k) * KYBER_POLY_SIZE)
5353
/* Size of a compressed polynomial based on bits per coefficient. */
54-
#define KYBER_POLY_COMPRESSED_SZ(b) (b * (KYBER_N / 8))
54+
#define KYBER_POLY_COMPRESSED_SZ(b) ((b) * (KYBER_N / 8))
5555
/* Size of a compressed vector polynomial based on dimensions and bits per
5656
* coefficient. */
57-
#define KYBER_POLY_VEC_COMPRESSED_SZ(k, b) (k * (b * (KYBER_N / 8)))
57+
#define KYBER_POLY_VEC_COMPRESSED_SZ(k, b) ((k) * ((b) * (KYBER_N / 8)))
5858

5959

6060
/* Kyber-512 parameters */

‎wolfssl/wolfcrypt/port/liboqs/liboqs.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,4 @@ int wolfSSL_liboqsRngMutexUnlock(void);
5757
} /* extern "C" */
5858
#endif
5959

60-
#endif /* WOLF_CRYPT_LIBOQS_H */
60+
#endif /* WOLF_CRYPT_LIBOQS_H */

0 commit comments

Comments
 (0)