fenrir fixes for closing fd, sanity check init calls, buffer scope, sanity check argmunts passed in

JacobBarthelmeh · JacobBarthelmeh · commit b19a33d7b207 · 2026-05-01T15:26:39.000-06:00
diff --git a/src/crypto/clu_decrypt.c b/src/crypto/clu_decrypt.c
@@ -217,8 +217,10 @@ int wolfCLU_decrypt(int alg, char* mode, byte* pwdKey, byte* key, int size,
                 wolfCLU_LogError("bad length %d found", length);
                 ret = -1;
             }
-            /* reset tempMax for smaller decryption */
-            XFWRITE(output, 1, length, outFile);
+            else {
+                /* reset tempMax for smaller decryption */
+                XFWRITE(output, 1, length, outFile);
+            }
         }
         else {
             if (output != NULL)
diff --git a/src/dh/clu_dh.c b/src/dh/clu_dh.c
@@ -381,6 +381,8 @@ int wolfCLU_DhParamSetup(int argc, char** argv)
     byte genKey = 0;
     byte check  = 0;
     byte noOut  = 0;
+    byte rngInited = 0;
+    byte dhInited  = 0;
     WOLFSSL_BIO *bioIn  = NULL;
     WOLFSSL_BIO *bioOut = NULL;
 
@@ -457,11 +459,20 @@ int wolfCLU_DhParamSetup(int argc, char** argv)
         }
     }
 
-    /* try initializing both because both get free'd regardless at the end */
-    if (wc_InitRng(&rng) != 0 || wc_InitDhKey(&dh) != 0) {
-        wolfCLU_LogError("Unable to initialize rng and dh");
+    if (wc_InitRng(&rng) != 0) {
+        wolfCLU_LogError("Unable to initialize rng");
         ret = WOLFCLU_FATAL_ERROR;
     }
+    else {
+        rngInited = 1;
+    }
+    if (ret == WOLFCLU_SUCCESS && wc_InitDhKey(&dh) != 0) {
+        wolfCLU_LogError("Unable to initialize dh");
+        ret = WOLFCLU_FATAL_ERROR;
+    }
+    else if (ret == WOLFCLU_SUCCESS) {
+        dhInited = 1;
+    }
 
     /* read in parameters */
     if (ret == WOLFCLU_SUCCESS && bioIn != NULL) {
@@ -789,8 +800,10 @@ int wolfCLU_DhParamSetup(int argc, char** argv)
     wolfSSL_BIO_free(bioIn);
     wolfSSL_BIO_free(bioOut);
 
-    wc_FreeDhKey(&dh);
-    wc_FreeRng(&rng);
+    if (dhInited)
+        wc_FreeDhKey(&dh);
+    if (rngInited)
+        wc_FreeRng(&rng);
 
     return ret;
 #else
diff --git a/src/dsa/clu_dsa.c b/src/dsa/clu_dsa.c
@@ -59,6 +59,8 @@ int wolfCLU_DsaParamSetup(int argc, char** argv)
     char* out = NULL;
     byte genKey = 0;
     byte noOut  = 0;
+    byte rngInited = 0;
+    byte dsaInited = 0;
     WOLFSSL_BIO *bioIn  = NULL;
     WOLFSSL_BIO *bioOut = NULL;
 
@@ -120,11 +122,20 @@ int wolfCLU_DsaParamSetup(int argc, char** argv)
         }
     }
 
-    /* try initializing both because both get free'd regardless at the end */
-    if (wc_InitRng(&rng) != 0 || wc_InitDsaKey(&dsa) != 0) {
-        wolfCLU_LogError("Unable to initialize rng and dsa");
+    if (wc_InitRng(&rng) != 0) {
+        wolfCLU_LogError("Unable to initialize rng");
         ret = WOLFCLU_FATAL_ERROR;
     }
+    else {
+        rngInited = 1;
+    }
+    if (ret == WOLFCLU_SUCCESS && wc_InitDsaKey(&dsa) != 0) {
+        wolfCLU_LogError("Unable to initialize dsa");
+        ret = WOLFCLU_FATAL_ERROR;
+    }
+    else if (ret == WOLFCLU_SUCCESS) {
+        dsaInited = 1;
+    }
 
     /* read in parameters */
     if (ret == WOLFCLU_SUCCESS && bioIn != NULL) {
@@ -342,8 +353,10 @@ int wolfCLU_DsaParamSetup(int argc, char** argv)
     wolfSSL_BIO_free(bioIn);
     wolfSSL_BIO_free(bioOut);
 
-    wc_FreeDsaKey(&dsa);
-    wc_FreeRng(&rng);
+    if (dsaInited)
+        wc_FreeDsaKey(&dsa);
+    if (rngInited)
+        wc_FreeRng(&rng);
 
     return ret;
 #else
diff --git a/src/ecparam/clu_ecparam.c b/src/ecparam/clu_ecparam.c
@@ -151,6 +151,7 @@ int wolfCLU_ecparam(int argc, char** argv)
                     break;
                 }
                 XSTRNCPY(name, optarg, ECC_MAXNAME);
+                name[ECC_MAXNAME - 1] = '\0';
 
                 /* convert name to upper case */
                 for (i = 0; i < (int)XSTRLEN(name); i++)
diff --git a/src/genkey/clu_genkey.c b/src/genkey/clu_genkey.c
@@ -1437,6 +1437,13 @@ int wolfCLU_genKey_ML_DSA(WC_RNG* rng, char* fName, int directive, int fmt,
                 XMEMCPY(fOutNameBuf + fNameSz, fExtPub, fExtSz);
                 WOLFCLU_LOG(WOLFCLU_L0, "Public key file = %s", fOutNameBuf);
 
+                /* free any prior derBuf (from PRIV path or initial alloc)
+                 * before reallocating for the public key */
+                if (derBuf != NULL) {
+                    wolfCLU_ForceZero(derBuf, keySz);
+                    XFREE(derBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+                    derBuf = NULL;
+                }
                 derBuf = (byte*)XMALLOC(keySz, HEAP_HINT,
                                 DYNAMIC_TYPE_TMP_BUFFER);
                 if (derBuf == NULL) {
@@ -1564,6 +1571,7 @@ enum wc_XmssRc wolfCLU_XmssKey_WriteCb(const byte * priv,
     if (n_write != privSz) {
         fprintf(stderr, "error: wrote %zu, expected %d: %d\n", n_write, privSz,
                 ferror(file));
+        fclose(file);
         return WC_XMSS_RC_WRITE_FAIL;
     }
 
@@ -1584,6 +1592,7 @@ enum wc_XmssRc wolfCLU_XmssKey_WriteCb(const byte * priv,
     buff = malloc(privSz);
     if (buff == NULL) {
         fprintf(stderr, "error: malloc(%d) failed\n", privSz);
+        fclose(file);
         return WC_XMSS_RC_WRITE_FAIL;
     }
 
@@ -1595,6 +1604,7 @@ enum wc_XmssRc wolfCLU_XmssKey_WriteCb(const byte * priv,
         fprintf(stderr, "error: read %zu, expected %zu: %d\n", n_read, n_write,
                 ferror(file));
         free(buff);
+        fclose(file);
         return WC_XMSS_RC_WRITE_FAIL;
     }
 
@@ -1604,6 +1614,7 @@ enum wc_XmssRc wolfCLU_XmssKey_WriteCb(const byte * priv,
 
     if (n_cmp != 0) {
         fprintf(stderr, "error: write data was corrupted: %d\n", n_cmp);
+        fclose(file);
         return WC_XMSS_RC_WRITE_FAIL;
     }
 
@@ -1641,6 +1652,7 @@ enum wc_XmssRc wolfCLU_XmssKey_ReadCb(byte * priv,
     if (n_read != privSz) {
         fprintf(stderr, "error: read %zu, expected %d: %d\n", n_read, privSz,
                 ferror(file));
+        fclose(file);
         return WC_XMSS_RC_READ_FAIL;
     }
 
diff --git a/src/genkey/clu_genkey_setup.c b/src/genkey/clu_genkey_setup.c
@@ -47,6 +47,11 @@ int wolfCLU_genKeySetup(int argc, char** argv)
 
     XMEMSET(keyOutFName, 0, MAX_FILENAME_SZ);
 
+    if (argc < 3) {
+        wolfCLU_LogError("ERROR: missing key type argument");
+        wolfCLU_genKeyHelp();
+        return USER_INPUT_ERROR;
+    }
     keyType = argv[2];
 
     ret = wc_InitRng(&rng);
diff --git a/src/pkcs/clu_pkcs8.c b/src/pkcs/clu_pkcs8.c
@@ -87,6 +87,7 @@ int wolfCLU_PKCS8(int argc, char** argv)
     char password[MAX_PASSWORD_SIZE];
     int passwordSz = MAX_PASSWORD_SIZE;
     byte* pass = NULL;
+    byte* stdinKeyBuf = NULL;
 
     opterr = 0; /* do not display unrecognized options */
     optind = 0; /* start at indent 0 */
@@ -155,35 +156,44 @@ int wolfCLU_PKCS8(int argc, char** argv)
 
     /* currently only supporting PKCS8 parsing, input is expected */
     if (ret == WOLFCLU_SUCCESS && bioIn == NULL) {
-        byte keyBuffer[MAX_STDINSZ];
         word32 keyLen = 0;
 
-        XMEMSET(keyBuffer, 0, MAX_STDINSZ);
-        keyLen = (int)XFREAD(keyBuffer, 1, sizeof(keyBuffer) - 1, stdin);
-        if (keyLen <= 0) {
-            WOLFCLU_LOG(WOLFCLU_E0, "Error reading private key from stdin");
-            ret = WOLFCLU_FATAL_ERROR;
+        /* Heap-allocate so the buffer outlives this block; the BIO created
+         * below stores a pointer to it and is freed at function exit. */
+        stdinKeyBuf = (byte*)XMALLOC(MAX_STDINSZ, HEAP_HINT,
+                DYNAMIC_TYPE_TMP_BUFFER);
+        if (stdinKeyBuf == NULL) {
+            ret = MEMORY_E;
         }
         else {
-            /* Null-terminate the key buffer */
-            keyBuffer[keyLen] = '\0';
+            XMEMSET(stdinKeyBuf, 0, MAX_STDINSZ);
+            keyLen = (int)XFREAD(stdinKeyBuf, 1, MAX_STDINSZ - 1, stdin);
+            if (keyLen <= 0) {
+                WOLFCLU_LOG(WOLFCLU_E0,
+                        "Error reading private key from stdin");
+                ret = WOLFCLU_FATAL_ERROR;
+            }
+            else {
+                /* Null-terminate the key buffer */
+                stdinKeyBuf[keyLen] = '\0';
 
-            bioIn = wolfSSL_BIO_new_mem_buf(keyBuffer, keyLen);
+                bioIn = wolfSSL_BIO_new_mem_buf(stdinKeyBuf, keyLen);
 
-            if (bioIn == NULL) {
-                wolfCLU_LogError("Unable to open pkcs8 file %s",
-                        optarg);
-                ret = MEMORY_E;
-            }
-            else if (pass == NULL) {
-            /* Reopen terminal since we might get password data
-             * from stdin later */
-            #ifdef USE_WINDOWS_API
-                if (freopen("CON", "r", stdin) == NULL) {
-            #else
-                if (freopen("/dev/tty", "r", stdin) == NULL) {
-            #endif
-                    ret = WOLFCLU_FATAL_ERROR;
+                if (bioIn == NULL) {
+                    wolfCLU_LogError("Unable to open pkcs8 file %s",
+                            optarg);
+                    ret = MEMORY_E;
+                }
+                else if (pass == NULL) {
+                /* Reopen terminal since we might get password data
+                 * from stdin later */
+                #ifdef USE_WINDOWS_API
+                    if (freopen("CON", "r", stdin) == NULL) {
+                #else
+                    if (freopen("/dev/tty", "r", stdin) == NULL) {
+                #endif
+                        ret = WOLFCLU_FATAL_ERROR;
+                    }
                 }
             }
         }
@@ -282,6 +292,10 @@ int wolfCLU_PKCS8(int argc, char** argv)
     wolfSSL_BIO_free(bioIn);
     wolfSSL_BIO_free(bioOut);
     wolfSSL_EVP_PKEY_free(pkey);
+    if (stdinKeyBuf != NULL) {
+        wolfCLU_ForceZero(stdinKeyBuf, MAX_STDINSZ);
+        XFREE(stdinKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+    }
 
     return ret;
 #else
diff --git a/src/x509/clu_request_setup.c b/src/x509/clu_request_setup.c
@@ -224,7 +224,7 @@ static int _wolfSSL_X509_extensions_print(WOLFSSL_BIO* bio, WOLFSSL_X509* x509,
                 XSTRLCAT(scratch, buf, MAX_WIDTH);
 
                 int crit = wolfSSL_X509_EXTENSION_get_critical(ext) ? 1 : 0;
-                XSTRLCAT(scratch, crit ? ": Critical\n" : ":\n", crit ? 11 : 2);
+                XSTRLCAT(scratch, crit ? ": Critical\n" : ":\n", MAX_WIDTH);
                 (void)crit;
 
                 wolfSSL_BIO_write(bio, scratch, (int)XSTRLEN(scratch));

Original file line number	Diff line number	Diff line change
`@@ -217,8 +217,10 @@ int wolfCLU_decrypt(int alg, char* mode, byte* pwdKey, byte* key, int size,`
`217`	`217`	`wolfCLU_LogError("bad length %d found", length);`
`218`	`218`	`ret = -1;`
`219`	`219`	`}`
`220`		`- /* reset tempMax for smaller decryption */`
`221`		`- XFWRITE(output, 1, length, outFile);`
	`220`	`+ else {`
	`221`	`+ /* reset tempMax for smaller decryption */`
	`222`	`+ XFWRITE(output, 1, length, outFile);`
	`223`	`+ }`
`222`	`224`	`}`
`223`	`225`	`else {`
`224`	`226`	`if (output != NULL)`
Original file line number	Diff line number	Diff line change
`@@ -151,6 +151,7 @@ int wolfCLU_ecparam(int argc, char** argv)`
`151`	`151`	`break;`
`152`	`152`	`}`
`153`	`153`	`XSTRNCPY(name, optarg, ECC_MAXNAME);`
	`154`	`+ name[ECC_MAXNAME - 1] = '\0';`
`154`	`155`
`155`	`156`	`/* convert name to upper case */`
`156`	`157`	`for (i = 0; i < (int)XSTRLEN(name); i++)`