20260729 Coverity fixes

[rlm2002]

Description

CID 561836: Untrusted value as argument - Bound untrusted DER length before allocation in wolfssl_read_der_bio
CID 561978: Uninitialized scalar variable - Initialize enc in test_coding.c to avoid use before set.
CID 562025: Data race condition - Lock globalRNG in AddSession when falling back to the global RNG

Testing

./configure --enable-jni && make check

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m4-openssl-compat

• FLASH: .rodata +24 B, .text +64 B (+0.0%, 772,164 B / 1,048,576 B, total: 74% used)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[Frauschi]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 12 total — 4 posted, 8 skipped

Posted findings

• [High] globalRNGMutex leaked when wc_RNG_GenerateBlock() fails in AddSession() — src/ssl_sess.c:2172-2185
• [Medium] initGlobalRNG is not re-checked after taking the lock (documented racy pattern elsewhere) — src/ssl_sess.c:2167-2177
• [Medium] No test exercises the new oversized-DER rejection path — src/pk_rsa.c:654-659
• [Low] test_coding.c change guards the read but does not initialize enc as the PR describes — tests/api/test_coding.c:328-347

Skipped findings

• [High] globalRNGMutex leaked on wc_RNG_GenerateBlock failure path in AddSession
• [High] globalRNGMutex left locked when wc_RNG_GenerateBlock() fails in AddSession()
• [Medium] DER size cap expression (RSA_MAX_SIZE / 8) * 8 reduces to RSA_MAX_SIZE and contradicts its comment
• [Medium] AddSession() global-RNG locking path has no regression test
• [Medium] initGlobalRNG not re-checked after acquiring globalRNGMutex in AddSession (races wolfSSL_RAND_Cleanup)
• [Low] Misindented return block does not match wolfSSL brace/indent style
• [Info] DER cap expression (RSA_MAX_SIZE / 8) * 8 is a no-op and its comment contradicts the code
• [Info] New error-return block in AddSession() uses non-conforming indentation

Review generated by Skoll via Claude/Codex

[Frauschi]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 7 total — 2 posted, 5 skipped

Posted findings

• [High] New test's preprocessor guard does not match the API it calls - link failure in OPENSSL_EXTRA-without-OPENSSL_ALL builds — tests/api.c:20494-20520
• [High] Test installs a raw malloc callback while leaving the previous free/realloc callbacks in place - invalid free in --enable-trackmemory builds — tests/api.c:20502-20551

Skipped findings

• [Medium] Bit-count constant RSA_MAX_SIZE compared against a byte length
• [Medium] Triple-duplicated unlock blocks and a garbled comment in AddSession
• [Medium] No test coverage for the new globalRNG locking path in AddSession
• [Low] enc[outLen - 1] still underflows if Base64_Encode returns outLen == 0
• [Low] Redundant #ifndef NO_BIO block in testCases[] and a free of an always-NULL pointer

Review generated by Skoll via Claude/Codex

[Frauschi]

LGTM

[dgarske]

Over to @philljj as he's reviewing another PR with similar global mutex fixes

[philljj]

I can't find a code path where ssl->rng would ever be null. Are we sure this isn't dead code?

We could also try to use a local rng first, and only fallback to the global RNG on error. We follow this pattern in src/pk.c and other places:

        int initTmpRng = 0; 
        WC_RNG *rng = NULL;
        WC_DECLARE_VAR(tmpRng, WC_RNG, 1, 0);
                                                                                
        WC_ALLOC_VAR_EX(tmpRng, WC_RNG, 1, NULL, DYNAMIC_TYPE_RNG,
            return WOLFSSL_FATAL_ERROR);
        if (wc_InitRng(tmpRng) == 0) {
            rng = tmpRng;
            initTmpRng = 1;
        }
        else {
            WOLFSSL_MSG("Bad RNG Init, trying global");
            rng = wolfssl_get_global_rng();
        }

Then we would only hit the global lock when the local temp rng init fails (which should be rare).

[philljj]

I was wrong, this branch is reachable per this other PR comment:

• Lock globalRNGMutex in BN_rand, ECDH, EC25519 and AddSession #11048 (review)

Both PR #11048 and this PR are making this same change to ssl_sess.c.

[philljj]

Merge conflict. Another PR (#11048) just fixed the same issue in src/ssl_sess.c