Fenrir fixes by aidankeefe2022 · Pull Request #255 · wolfSSL/wolfCLU

aidankeefe2022 · 2026-06-22T18:01:33Z

Added stdout configuration to rand when running on windows to make it behave like Posix

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #255

Scan targets checked: wolfclu-bugs, wolfclu-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.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #255

Scan targets checked: wolfclu-bugs, wolfclu-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.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #255

Scan targets checked: wolfclu-bugs, wolfclu-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.

Copilot

Pull request overview

This PR addresses multiple Fenrir findings by hardening CLI argument handling and file-length processing, adds regression tests for those cases, and fixes Windows rand stdout behavior to avoid output corruption.

Changes:

Add regression tests for verify/dgst malformed-argument edge cases (missing trailing positional args) and a verify no-crash case.
Switch Windows rand stdout to binary mode when emitting to stdout to prevent newline translation from corrupting output.
Improve robustness around buffer sizing and OCSP index parsing (e.g., safer serial handling, larger size types, and negative-length checks).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/x509/x509-verify-test.py	Adds regression tests for `verify` malformed-argument and no-crash scenarios.
tests/dgst/dgst-test.py	Adds regression tests ensuring `dgst` detects missing trailing data file and doesn’t mis-flag valid invocations.
src/tools/clu_rand.c	Sets stdout to binary mode on Windows when writing to stdout to avoid data corruption.
src/sign-verify/clu_x509_verify.c	Adds malformed-argument detection before loading certs.
src/sign-verify/clu_verify.c	Uses `long` for file sizes and adds negative-size checks before allocating/reading.
src/sign-verify/clu_dgst_setup.c	Adjusts option parsing to avoid scanning the trailing positional and adds malformed-argument detection.
src/ocsp/clu_ocsp.c	Enlarges serial buffer and adds length validation + explicit NUL termination.
src/dh/clu_dh.c	Uses `long` for BIO length and casts for read/decode.
src/crypto/clu_evp_crypto.c	Uses `long` for BIO length and checks for negative length before base64 decode.

Comments suppressed due to low confidence (1)

src/dh/clu_dh.c:488

wolfSSL_BIO_get_len() can return 0/negative for empty or unreadable input. With the current logic, that case silently skips parsing the input params and continues, which can lead to confusing downstream failures. Treat non-positive lengths as an error before proceeding.

        inSz = wolfSSL_BIO_get_len(bioIn);
        if (inSz > 0) {
            in = (byte*)XMALLOC(inSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
            if (in == NULL) {
                ret = WOLFCLU_FATAL_ERROR;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

https://fenrir.wolfssl.com/finding/5020 https://fenrir.wolfssl.com/finding/5022 https://fenrir.wolfssl.com/finding/5019 https://fenrir.wolfssl.com/finding/5021 https://fenrir.wolfssl.com/finding/5018 make windows behave and not add random bytes!!! copilot fixes

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

src/sign-verify/clu_verify.c:346

keyFileSz is now a long, but it is still used in (int)XFREAD(...) comparisons and cast to int when calling wolfCLU_KeyPemToDer. Add an explicit keyFileSz > INT_MAX check and avoid truncating XFREAD's return value to int to prevent confusing failures on large inputs.

    if (ret == 0) {
        XFSEEK(keyPathFile, 0, SEEK_END);
        keyFileSz = XFTELL(keyPathFile);
        if (keyFileSz < 0) {
            wolfCLU_LogError("Unable to Get Size of Key File %s.", keyPath);
            ret = BAD_FUNC_ARG;
        }
    }
    if (ret == 0) {
        keyBuf = (byte*)XMALLOC(keyFileSz+1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
        if (keyBuf == NULL) {
            ret = MEMORY_E;
        }
    }
    if (ret == 0) {
        XMEMSET(keyBuf, 0, keyFileSz+1);
        if (XFSEEK(keyPathFile, 0, SEEK_SET) != 0 ||
            (int)XFREAD(keyBuf, 1, keyFileSz, keyPathFile) != keyFileSz) {
            ret = WOLFCLU_FATAL_ERROR;

src/dh/clu_dh.c:485

inSz is widened to long but is then cast to int for wolfSSL_BIO_read and wc_DhKeyDecode without any upper-bound validation. If inSz exceeds INT_MAX, these casts can truncate and lead to short reads/decodes. Validate that inSz fits in an int before using it in int-based APIs.

        long inSz = 0;
        word32 idx  = 0;

        inSz = wolfSSL_BIO_get_len(bioIn);
        if (inSz > 0) {

+    def test_last_arg_not_misread(self):
+        r = run_wolfssl("verify", "-partial_chain", "-CAfile",
+                        os.path.join(CERTS_DIR, "server-cert.pem"),
+                        "-untrusted", os.path.join(CERTS_DIR, "random.pem")
+                        )
+        self.assertNotEqual(r.returncode, 0, r.stderr)


+        r = run_wolfssl("dgst", "-sha256", "-verify",
+                        os.path.join(CERTS_DIR, "server-keyPub.pem"),
+                        "-signature", os.path.join(DGST_DIR, "sha256-rsa.sig"))
+        self.assertNotEqual(r.returncode, 0, r.stderr)


philljj · 2026-07-09T16:46:32Z

 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
 */

+#include "wolfclu/clu_error_codes.h"


We do always include it bracketed:

$ grep -r "include.*clu_error_codes\.h" * --include=*.c src/clu_main.c:#include <wolfclu/clu_error_codes.h> src/ocsp/clu_ocsp.c:#include <wolfclu/clu_error_codes.h> src/tools/clu_base64.c:#include <wolfclu/clu_error_codes.h> src/x509/clu_cert_setup.c:#include <wolfclu/clu_error_codes.h> src/x509/clu_config.c:#include <wolfclu/clu_error_codes.h> src/x509/clu_parse.c:#include <wolfclu/clu_error_codes.h> src/x509/clu_x509_sign.c:#include <wolfclu/clu_error_codes.h>

+    if (argc >= 2 && optarg != NULL && XSTRCMP(optarg, argv[argc-1]) == 0) {
+        wolfCLU_LogError("Malformed arguments last argument read as value for "
+                "%s", argv[argc-2]);
+        ret = WOLFCLU_FATAL_ERROR;
+    }


+    if (argc >= 2 && optarg != NULL && XSTRCMP(optarg, argv[argc-1]) == 0 &&
+            (partialChain == 0 || caCert == NULL ||
+             XSTRCMP(caCert, argv[argc-1]) != 0) &&
+             (intermCert == NULL || XSTRCMP(intermCert, argv[argc-1]) != 0)) {
+        wolfCLU_LogError("Malformed arguments last argument read as value for "
+                "%s", argv[argc-2]);
        ret = WOLFCLU_FATAL_ERROR;
    }


    XFSEEK(f, 0, SEEK_END);
-    fSz = (int)XFTELL(f);
+    fSz = XFTELL(f);
+
+    if (fSz < 0) {
+        wolfCLU_LogError("Invalid Sig File %s.", sig);


            XFSEEK(h, 0, SEEK_END);
-            hSz = (int)XFTELL(h);
+            hSz = XFTELL(h);
+
+            if (hSz < 0) {
+                wolfCLU_LogError("Unable to Get Size of Hash File %s.",


philljj · 2026-07-09T16:31:19Z

Merge conflict in src/sign-verify/clu_dgst_setup.c.

Copilot found a few things, but some are probably too nitpicky and can be ignored and resolved (leave a comment explaining if doing nothing).

philljj · 2026-07-09T16:48:03Z

        }
    }

+    /* Detect malformed arguments: if the trailing positional data file was


What is this trying to detect? Is there a more straightforward way to err check input?

aidankeefe2022 requested a review from wolfSSL-Fenrir-bot June 22, 2026 18:01

aidankeefe2022 self-assigned this Jun 22, 2026

aidankeefe2022 force-pushed the fenrir-fixes-jun18-26 branch from ee33f0a to 546a0b8 Compare June 22, 2026 18:20

wolfSSL-Fenrir-bot reviewed Jun 22, 2026

View reviewed changes

Comment thread src/ocsp/clu_ocsp.c Outdated

aidankeefe2022 commented Jun 22, 2026

View reviewed changes

Comment thread src/ocsp/clu_ocsp.c Outdated

aidankeefe2022 force-pushed the fenrir-fixes-jun18-26 branch 4 times, most recently from 5468838 to 963c0c7 Compare June 23, 2026 16:43

aidankeefe2022 requested a review from wolfSSL-Fenrir-bot June 23, 2026 16:45

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/sign-verify/clu_verify.c Outdated

aidankeefe2022 force-pushed the fenrir-fixes-jun18-26 branch from 963c0c7 to b7acc6b Compare June 23, 2026 17:41

aidankeefe2022 commented Jun 23, 2026

View reviewed changes

Comment thread src/sign-verify/clu_verify.c Outdated

aidankeefe2022 removed their assignment Jun 23, 2026

aidankeefe2022 requested a review from wolfSSL-Fenrir-bot June 23, 2026 18:02

aidankeefe2022 self-assigned this Jun 23, 2026

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/sign-verify/clu_x509_verify.c Outdated

aidankeefe2022 force-pushed the fenrir-fixes-jun18-26 branch 2 times, most recently from 0e179fb to 5edf352 Compare June 23, 2026 21:14

aidankeefe2022 assigned cconlon and unassigned aidankeefe2022 Jun 23, 2026

cconlon requested a review from Copilot June 23, 2026 22:43

Copilot started reviewing on behalf of cconlon June 23, 2026 22:43 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread tests/x509/x509-verify-test.py Outdated

Comment thread src/sign-verify/clu_x509_verify.c Outdated

Comment thread src/crypto/clu_evp_crypto.c Outdated

Comment thread src/sign-verify/clu_verify.c

cconlon assigned aidankeefe2022 and unassigned cconlon Jun 24, 2026

aidankeefe2022 force-pushed the fenrir-fixes-jun18-26 branch from 5edf352 to 461205c Compare June 25, 2026 17:15

aidankeefe2022 assigned cconlon and unassigned aidankeefe2022 Jun 25, 2026

dgarske assigned wolfSSL-Bot and philljj and unassigned cconlon Jul 7, 2026

dgarske requested a review from philljj July 7, 2026 21:39

philljj requested a review from Copilot July 9, 2026 16:16

Copilot started reviewing on behalf of philljj July 9, 2026 16:17 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

philljj assigned aidankeefe2022 and unassigned wolfSSL-Bot Jul 9, 2026

philljj reviewed Jul 9, 2026

View reviewed changes

philljj removed their assignment Jul 9, 2026

Uh oh!

Conversation

aidankeefe2022 commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #255

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #255

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #255

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

philljj Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philljj commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philljj Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aidankeefe2022 commented Jun 22, 2026 •

edited

Loading

philljj Jul 9, 2026 •

edited

Loading

philljj commented Jul 9, 2026 •

edited

Loading