Skip to content

Fixes for buffer bounds, error handling, and key material cleanup - #257

Open
cconlon wants to merge 10 commits into
wolfSSL:masterfrom
cconlon:fenrirAug13_2
Open

Fixes for buffer bounds, error handling, and key material cleanup#257
cconlon wants to merge 10 commits into
wolfSSL:masterfrom
cconlon:fenrirAug13_2

Conversation

@cconlon

@cconlon cconlon commented Aug 14, 2026

Copy link
Copy Markdown
Member

This PR includes 10 Fenrir fixes:

  • F-6904: Return false from LMS verify wrapper for wrong length sigs to match the JCE Signature.verify() contract
  • F-8197: Normalize X509_STORE_add_cert failure codes to negative so Java does not read compat layer failure values as success
  • F-8198: Check ECC PKCS8 size query result before allocating the output buffer
  • F-8199: Fix curve size sign handling in ECC import_public_raw coordinate length validation
  • F-8205: Bound the direct ByteBuffer write position in the Md5 final wrapper
  • F-6437: Zeroize copied ML-KEM secret inputs (seeds, random, private keys) before JNI array release, with shared secret input helpers added to jni_native_struct.c
  • F-6902: Zeroize copied SLH-DSA secret inputs before JNI array release using the shared helpers
  • F-6903: Bound the direct ByteBuffer write position in SHA/SHA-224/SHA-256/SHA-384/SHA-512/SHA-3 final wrappers, with regression tests for each
  • F-8206: Check GetStringUTFChars results in ECC curve name paths before use
  • F-3561: Call wc_AesInit/wc_Des3Init at allocation and wc_AesFree/wc_Des3Free in releaseNativeStruct() for Aes, AesCtr, AesEcb, AesOfb, and Des3 so key schedules are zeroized before the
    native struct is freed

@cconlon cconlon self-assigned this Aug 14, 2026
Copilot AI lite review requested due to automatic review settings August 14, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the wolfCrypt JNI/JCE bindings by adding bounds checks for direct ByteBuffer digest finalization, normalizing/aligning error handling with Java/JCE contracts, and ensuring key/secret material is properly zeroized before native structures are freed.

Changes:

  • Add direct ByteBuffer capacity/position validation in MD5 and SHA/SHA-2/SHA-3 native_final JNI wrappers, plus regression tests that assert invalid positions throw.
  • Introduce shared JNI helpers to zeroize copied secret byte-array inputs before releasing them, and apply them to ML-KEM and SLH-DSA private/seed inputs.
  • Ensure AES/DES3 native structs are initialized at allocation and have their internal key schedules zeroized via wc_*Free() prior to freeing the native memory; align LMS verify behavior to return false for wrong-length signatures.

Reviewed changes

Copilot reviewed 29 out of 34 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/test/java/com/wolfssl/wolfcrypt/test/ShaTest.java Adds regression test for invalid native_final(ByteBuffer, position) bounds handling.
src/test/java/com/wolfssl/wolfcrypt/test/Sha224Test.java Adds regression test for invalid native_final position handling.
src/test/java/com/wolfssl/wolfcrypt/test/Sha256Test.java Adds regression test for invalid native_final position handling.
src/test/java/com/wolfssl/wolfcrypt/test/Sha384Test.java Adds regression test for invalid native_final position handling.
src/test/java/com/wolfssl/wolfcrypt/test/Sha512Test.java Adds regression test for invalid native_final position handling.
src/test/java/com/wolfssl/wolfcrypt/test/Sha3Test.java Adds regression test for invalid native_final position handling for SHA3-256.
src/test/java/com/wolfssl/wolfcrypt/test/Md5Test.java Adds regression test for invalid native_final position handling.
src/test/java/com/wolfssl/wolfcrypt/test/LmsTest.java Adds test ensuring truncated LMS signatures verify as false (not exceptions).
src/test/java/com/wolfssl/provider/jce/test/WolfCryptLmsSignatureTest.java Adds JCE-level test ensuring truncated LMS signatures return false.
src/main/java/com/wolfssl/wolfcrypt/BlockCipher.java Adds nativeFree() hook and calls it under pointerLock before freeing native structs.
src/main/java/com/wolfssl/wolfcrypt/Aes.java Overrides nativeFree() to call wc_AesFree() for key schedule zeroization.
src/main/java/com/wolfssl/wolfcrypt/AesEcb.java Overrides nativeFree() to call wc_AesFree() for key schedule zeroization.
src/main/java/com/wolfssl/wolfcrypt/Des3.java Overrides nativeFree() to call wc_Des3Free() for key schedule zeroization.
src/main/java/com/wolfssl/wolfcrypt/AesCtr.java Calls wc_AesFree() under pointerLock in releaseNativeStruct() for key schedule zeroization.
src/main/java/com/wolfssl/wolfcrypt/AesOfb.java Calls wc_AesFree() under pointerLock in releaseNativeStruct() for key schedule zeroization.
jni/jni_sha.c Adds direct-buffer capacity checks to prevent out-of-bounds writes during finalization (SHA/SHA-2/SHA-3).
jni/jni_md5.c Adds direct-buffer capacity checks to prevent out-of-bounds writes during finalization (MD5).
jni/jni_native_struct.c Adds secret-array pin/release helpers that zeroize copied secret inputs before releasing.
jni/include/wolfcrypt_jni_NativeStruct.h Exposes new getSecretByteArray / releaseSecretByteArray helper declarations.
jni/jni_mlkem.c Uses secret-array helpers to zeroize copied ML-KEM secret inputs before releasing arrays.
jni/jni_slhdsa.c Uses secret-array helpers to zeroize copied SLH-DSA secret inputs before releasing arrays.
jni/jni_lms.c Treats wrong-length signatures as verification failure (false) rather than throwing.
jni/jni_wolfssl_x509_store_ctx.c Normalizes non-negative failure codes from X509_STORE_add_cert to negative error codes.
jni/jni_ecc.c Adds null checks for GetStringUTFChars, fixes curve size sign handling, and guards PKCS#8 allocation on successful size query.
jni/jni_aes.c Initializes AES at allocation (wc_AesInit) and adds JNI wrapper for wc_AesFree.
jni/jni_aesecb.c Initializes AES-ECB at allocation and adds JNI wrapper for wc_AesFree.
jni/jni_aesctr.c Initializes AES-CTR at allocation and adds JNI wrapper for wc_AesFree.
jni/jni_aesofb.c Initializes AES-OFB at allocation and adds JNI wrapper for wc_AesFree.
jni/jni_des3.c Initializes DES3 at allocation (wc_Des3Init) and adds JNI wrapper for wc_Des3Free.
jni/include/com_wolfssl_wolfcrypt_Aes.h Declares JNI wc_AesFree() method.
jni/include/com_wolfssl_wolfcrypt_AesEcb.h Declares JNI wc_AesFree() method.
jni/include/com_wolfssl_wolfcrypt_AesCtr.h Declares JNI wc_AesFree() method.
jni/include/com_wolfssl_wolfcrypt_AesOfb.h Declares JNI wc_AesFree() method.
jni/include/com_wolfssl_wolfcrypt_Des3.h Declares JNI wc_Des3Free() method.
Files not reviewed (5)
  • jni/include/com_wolfssl_wolfcrypt_Aes.h: Generated file
  • jni/include/com_wolfssl_wolfcrypt_AesCtr.h: Generated file
  • jni/include/com_wolfssl_wolfcrypt_AesEcb.h: Generated file
  • jni/include/com_wolfssl_wolfcrypt_AesOfb.h: Generated file
  • jni/include/com_wolfssl_wolfcrypt_Des3.h: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cconlon cconlon assigned rlm2002 and unassigned cconlon Aug 14, 2026
Comment thread jni/jni_lms.c Outdated
result = JNI_TRUE;
}
else if (ret != SIG_VERIFY_E) {
else if (ret != SIG_VERIFY_E && ret != BUFFER_E) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix on F-6904 for the wrong-length case. One more malformed-signature path lands in this same else branch though, so verify() still throws rather than returning false for it.

A correct-length signature whose embedded LMS type field has been altered returns SIG_TYPE_E (-231), not SIG_VERIFY_E or BUFFER_E. wc_LmsKey_Verify passes the length check (sigSz == params->sig_len) and calls wc_hss_verify, then wc_lms_verify, which compares the signature's LMS type word against the key's type and returns SIG_TYPE_E on mismatch (wc_lms_impl.c, the if (sigType != (params->lmsType & LMS_H_W_MASK)) check). That code propagates unchanged back to here, and since SIG_TYPE_E is neither SIG_VERIFY_E nor BUFFER_E, the else branch calls throwWolfCryptExceptionFromError. So a malformed-but-correct-length signature still throws, where the wrong-length one now returns false after F-6904. Both are malformed signatures, so the two paths behave inconsistently, and extending the same handling to SIG_TYPE_E would make verify() return false here too.

A single byte flip in the signature's LMS type field reproduces it. For a single-level key that word sits at offset LMS_L_LEN + LMS_Q_LEN + LMS_TYPE_LEN + hash_len + p*hash_len from the start of the signature, i.e. 1132 for SHA256_M32 with W8, where the leading LMS_L_LEN is the HSS levels field that wc_hss_verify consumes before wc_lms_verify. The length stays valid, the type word no longer matches the key, and verify() throws instead of returning false. The new truncated-signature test covers the length case well; a companion test that corrupts the type field and asserts verify() returns false would cover this one.

For consistency with the length case, it may be cleanest to treat the whole malformed-signature family uniformly: SIG_TYPE_E for certain, and it is worth checking whether an unsupported or otherwise invalid embedded type can surface yet another code, so that verify() returns false for any malformed signature and only throws on genuine operational errors (MEMORY_E, BAD_STATE_E, and so on). I traced this while looking at how LMS verify handles malformed input, and am glad to help with a repro or the extra test if useful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Arpan0995! I made this fix and squashed into the commit for F-6904 (98cefc5).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants