Update PQC groups for latest wolfSSL#209
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #209
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 1
High (1)
server.c usage message array element count mismatch shifts subsequent indices
File: src/server/server.c:1048-1058
Function: N/A (static array initialization)
Category: Logic errors
In server.c, the PQC help text in server_usage_msg[][65] was previously a single array element (using C string literal concatenation — no commas between adjacent string literals). The new code introduces commas between the string literals, splitting it into 4 separate array elements instead of 1. This adds 3 extra elements to both the English (around line 1048) and Japanese (around line 1234) sub-arrays. Since the usage printing code accesses elements by sequential index (msg[++msgId]), all entries after the PQC section (index 60) will be shifted by 3, causing misaligned help text output. Additionally, if the array's second dimension of 65 was previously at or near capacity, the 3 extra elements per language could cause an array out-of-bounds overflow.
For comparison, client.c correctly maintains the same element count (4 elements before and after the change) by preserving string concatenation for the later lines. The server.c change appears to be a copy-paste error where commas were inadvertently added between string literals that should have been concatenated.
/* New server.c English - 4 elements where old had 1 */
"--pqc <alg> Key Share with specified post-quantum algorithm only [ML_KEM_512, ML_KEM_768,\n", /* 60 */
" ML_KEM_1024, SecP256r1MLKEM512, SecP384r1MLKEM768, SecP256r1MLKEM768,\n",
" SecP521r1MLKEM1024, SecP384r1MLKEM1024, X25519MLKEM512, X25519MLKEM768,\n",
" X448MLKEM768, KYBER_LEVEL1, KYBER_LEVEL3, KYBER_LEVEL5, P256_KYBER_LEVEL1,\n"
" P384_KYBER_LEVEL3, P256_KYBER_LEVEL3, P521_KYBER_LEVEL5, X25519_KYBER_LEVEL1,\n"
" X25519_KYBER_LEVEL3, X448_KYBER_LEVEL3]\n",
/* Old server.c English - 1 element using string concatenation */
"--pqc <alg> Key Share with specified post-quantum algorithm only [KYBER_LEVEL1, KYBER_LEVEL3,\n"
" KYBER_LEVEL5, P256_KYBER_LEVEL1, P384_KYBER_LEVEL3, P521_KYBER_LEVEL5] \n", /* 60 */Recommendation: Use string literal concatenation (no commas between adjacent string literals) to keep the PQC help text as a single array element, matching how client.c handles it. For the English section, remove the commas after lines 2 and 3 so they concatenate into a single element. Apply the same fix to the Japanese section. For example:
"--pqc <alg> Key Share ... [ML_KEM_512, ML_KEM_768,\n"
" ML_KEM_1024, SecP256r1MLKEM512, ...\n"
" SecP521r1MLKEM1024, ...\n"
" X448MLKEM768, ... X448_KYBER_LEVEL3]\n", /* 60 */This review was generated automatically by Fenrir. Findings are non-blocking.
|
The fenrir finding is a false-positive, as the usage() print logic within the server has been broken before. Now it is correct. |
All the other KEM options have already been removed from upstream wolfssl for a long time. This is now equivalent to the client and server examples of wolfssl.