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%, 771,500 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]

🟠 [High] New test's preprocessor guard does not match the API it calls - link failure in OPENSSL_EXTRA-without-OPENSSL_ALL builds
🚫 BLOCK bug

The new test body is guarded on defined(OPENSSL_EXTRA) && !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) && ..., but the function it calls, d2i_RSAPrivateKey_bio -> wolfSSL_d2i_RSAPrivateKey_bio, is compiled only under a different guard. In src/pk_rsa.c:622-625 the definition is wrapped in #if defined(OPENSSL_ALL) || defined(WOLFSSL_ASIO) || defined(WOLFSSL_HAPROXY) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_QT) plus #if defined(WOLFSSL_KEY_GEN) && !defined(NO_BIO), and the prototype in wolfssl/ssl.h:1567-1587 carries the same guard. OPENSSL_EXTRA does not imply OPENSSL_ALL anywhere in configure.ac or settings.h (only the reverse holds). The d2i_RSAPrivateKey_bio macro in wolfssl/openssl/ssl.h:1398 is defined unconditionally, so the call site compiles (with an implicit-declaration warning, which is an error under the -Werror CI configs) and then fails at link with an undefined reference to wolfSSL_d2i_RSAPrivateKey_bio. Realistic configurations that hit this: --enable-wpas (configure.ac:3118-3144 sets -DWOLFSSL_KEY_GEN and -DOPENSSL_EXTRA but never -DOPENSSL_ALL; used in .github/workflows/hostap-vm.yml:59), --enable-fortress combined with --enable-opensslextra, and plain --enable-opensslextra --enable-keygen. The author's stated test config --enable-jni masks this because configure.ac:9435-9439 force-enables OPENSSL_ALL for JNI. Compare the sibling test in the same file, test_wolfSSL_d2i_PrivateKeys_bio, which is correctly guarded on OPENSSL_ALL || (WOLFSSL_ASIO && !NO_RSA).

Verified by building the PR head: ./configure --enable-opensslextra --enable-keygen fails with tests/api.c:20542:16: error: use of undeclared identifier 'd2i_RSAPrivateKey_bio'.

Suggestion:

Suggested change

	#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) && \
	#if (defined(OPENSSL_ALL) || defined(WOLFSSL_ASIO) || \
	defined(WOLFSSL_HAPROXY) || defined(WOLFSSL_NGINX) || \
	defined(WOLFSSL_QT)) && \
	!defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) && \
	defined(USE_WOLFSSL_MEMORY) && !defined(WOLFSSL_NO_MALLOC) && \
	!defined(WOLFSSL_STATIC_MEMORY) && !defined(WOLFSSL_DEBUG_MEMORY) && \
	!defined(WOLFSSL_TRACK_MEMORY)

[Frauschi]

🟠 [High] Test installs a raw malloc callback while leaving the previous free/realloc callbacks in place - invalid free in --enable-trackmemory builds
🚫 BLOCK bug

wolfSSL_SetAllocators(big_malloc_cb, prev_fc, prev_rc) swaps in a malloc hook that returns memory from the C library malloc(), but keeps the previous free and realloc callbacks installed. That breaks the allocator triple whenever the previous allocators are not plain malloc/free/realloc. This is exactly the case in a --enable-trackmemory build: configure.ac:5955-5965 defines only WOLFSSL_TRACK_MEMORY (no WOLFSSL_DEBUG_MEMORY, which is the only tracking-related macro the guard excludes), and wolfCrypt_Init() at wolfcrypt/src/wc_port.c:525-531 calls InitMemoryTracker(), which installs TrackMalloc/TrackFree/TrackRealloc globally (wolfssl/wolfcrypt/mem_track.h:381). TrackMalloc over-allocates by sizeof(memoryTrack) and returns an interior pointer; TrackFree (mem_track.h:243-290) does mt = (memoryTrack*)((byte*)ptr - sizeof(memoryTrack)), reads header->thisSize, and on Linux/macOS (DO_MEM_LIST) also unlinks header from ourMemList before free(mt). Concrete failure: with ./configure --enable-all --enable-trackmemory && make check, the test's own ExpectNotNull(bio = BIO_new(BIO_s_mem())) allocates through big_malloc_cb (raw malloc), then BIO_free(bio) at line 20547 routes to prev_fc == TrackFree, which frees a pointer 32+ bytes before a valid heap block and writes through garbage next/prev pointers - heap corruption / crash, not just bad statistics. The existing precedent this test was modeled on, test_write_dup_oom (tests/api.c:35660-35662), correctly replaces all three callbacks with oom_malloc_cb/oom_free_cb/oom_realloc_cb, which are consistent plain malloc/free/realloc wrappers.

Verified by building and running the PR head: ./configure --enable-opensslall --enable-keygen --enable-trackmemory builds cleanly, then ./tests/unit.test test_wolfSSL_d2i_RSAPrivateKey_bio_oversized segfaults (exit 139). Crash backtrace: TrackFree <- wolfSSL_BIO_free <- test_wolfSSL_d2i_RSAPrivateKey_bio_oversized -- the BIO is allocated by the raw-malloc hook and freed by TrackFree, which dereferences a memoryTrack header that was never written.

Suggestion:

Suggested change

	static void* big_malloc_cb(size_t size)
	/* Supply a matching free/realloc so the allocator triple stays consistent. */
	static void big_free_cb(void* ptr)
	{
	free(ptr);
	}
	static void* big_realloc_cb(void* ptr, size_t size)
	{
	if (big_alloc_threshold != 0 && size >= big_alloc_threshold) {
	big_alloc_attempts++;
	return NULL;
	}
	return realloc(ptr, size);
	}
	...
	ExpectIntEQ(wolfSSL_SetAllocators(big_malloc_cb, big_free_cb,
	big_realloc_cb), 0);