Memory and Secret Handling Fixes#589
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #589
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
stenslae
force-pushed
the
pr-memory-crypto
branch
from
9e04469 to
d324400
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #589
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
No new issues found in the changed files. ✅
stenslae
force-pushed
the
pr-memory-crypto
branch
from
d324400 to
34ccbb9
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #589
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
No new issues found in the changed files. ✅
stenslae
force-pushed
the
pr-memory-crypto
branch
from
34ccbb9 to
0a1c02d
Compare
…s, malloc/realloc NULL checks, and padding validation in crypto file-encryption examples
… F-6288: fix NULL-deref, fd-leak, unaligned-access, and buffer-overflow bugs across CAN, PKCS7, embedded, and PEM-printing examples
stenslae
force-pushed
the
pr-memory-crypto
branch
2 times, most recently
from
c4beee5 to
76b1000
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #589
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
stenslae
force-pushed
the
pr-memory-crypto
branch
from
76b1000 to
d490b56
Compare
…ce hardcoded/UAF-prone secrets and keys with RNG-generated values, zeroize key material before free, and flag non-constant-time secret comparisons
stenslae
force-pushed
the
pr-memory-crypto
branch
from
d490b56 to
e399570
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #589
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER); | ||
| fclose(inFile); | ||
| fclose(outFile); | ||
| } |
There was a problem hiding this comment.
🔵 [Low] fclose(outFile) without NULL check in new error paths across crypto file tools · Incorrect error handling
The PR adds key-malloc-failure and NoEcho-failure error paths that call fclose(outFile) without a NULL guard. outFile stays NULL when fopen(out, "w") fails (e.g. bad output directory) while outCheck is already set to 1, so the existing guard inCheck==0||outCheck==0 does not fire. fclose(NULL) is UB and crashes on Linux. Same issue in aescfb-file-encrypt.c, aesctr-file-encrypt.c, and camellia-encrypt.c; 3des-file-encrypt.c (line 469/483) is additionally missing the inFile!=NULL check on the outer else-if that the AES variants gained from this PR, so fclose(inFile) can also be NULL there.
Fix: Guard both new fclose calls: if (inFile != NULL) fclose(inFile); and if (outFile != NULL) fclose(outFile);. For 3des-file-encrypt.c and camellia-encrypt.c also add && inFile != NULL && outFile != NULL to the outer else-if condition.
| goto cleanup; | ||
| if ((unsigned int)(fileSize - 4 - SESSION_SALT_SZ - GCM_IV_SZ - GCM_TAG_SZ) != originalSz) { | ||
| XPRINTF("Error: File size does not match expected encrypted data size\n"); | ||
| fclose(fp); |
There was a problem hiding this comment.
🔵 [Low] Derived key not zeroed on file-size mismatch error path in LoadEncryptedSession · Missing error handling
DeriveSessionKey succeeds and fills the 32-byte stack buffer key with PBKDF2-derived key material (line 322), but the file-size validation failure at line 330 takes a bare return NULL without calling wc_ForceZero(key, sizeof(key)), leaving derived key material on the stack.
Fix: Replace the bare return NULL with goto cleanup so the existing wc_ForceZero(key, sizeof(key)) at the cleanup label is reached.
| @@ -303,11 +304,20 @@ int encrypt_file_AesGCM(const char *in_file, const char *out_file, | |||
| } | |||
| printf("File encryption with AES GCM complete.\n"); | |||
| exit: | |||
There was a problem hiding this comment.
🔵 [Low] wc_AesFree not called for stack Aes object in encrypt_file_AesGCM and decrypt_file_AesGCM · Resource leaks
The PR extends the exit: cleanup blocks in both encrypt_file_AesGCM and decrypt_file_AesGCM to add wc_ForceZero for key material and unlink on error, but omits wc_AesFree(&gcm). wc_AesInit is explicitly called at line 240 for encrypt_file_AesGCM, leaving the AES key schedule in the stack-allocated gcm struct unreleased and unzeroed.
Fix: Add wc_AesFree(&gcm) to both exit: cleanup labels, guarded by a flag set after wc_AesInit succeeds.
3 participants
27 Fenrir fixes across wolfSSL example code in three commits: