Skip to content

fix lareger(>57 octets) CRL number#9873

Merged
douzzer merged 4 commits intowolfSSL:masterfrom
miyazakh:fix_larger_crlnum
Mar 7, 2026
Merged

fix lareger(>57 octets) CRL number#9873
douzzer merged 4 commits intowolfSSL:masterfrom
miyazakh:fix_larger_crlnum

Conversation

@miyazakh
Copy link
Contributor

@miyazakh miyazakh commented Mar 5, 2026

Description

This is an additional fix for PR9628. There is an additional vulnerability triggered by a CRL number larger than 56 octests.

In ParseCRL_Extensions(ASN_TEMPLATE code path), the CRL number Integer is parsed by calling:

    GetInt(m, buf, &localIdx, maxIdx);
  Internally, the call chain is:
    GetInt
     └─ GetASN_Items        (asn.c)
         └─ GetASN_StoreData (asn.c)
             ├─ mp_init(data->data.mp)        ← OVERWRITES m->size with
             │                                   SP_INT_DIGITS (e.g. 130)
             └─ sp_read_unsigned_bin()        ← STACK OVERFLOW HERE

For a CRL Number larger than 56 bytes, sp_read_unsigned_bin writes past the end of the 64-byte stack allocation, overflowing into adjacent stack frames.

The stack over flow can be observer with the following build configuration:

./configure --enable-crl 'CFLAGS=-DHAVE_CRL_UPDATE_CB -fsanitize=address -fno-omit-frame-pointer -g'

Then run the unit test test_wolfSSL_CTX_LoadCRL_largeCRLnum() (after modifying the CRL number to be greater than 56 octets).

Fix: Add a length pre-check before calling GetInt() to reject CRL Numbers
exceeding CRL_MAX_NUM_SZ (20 octets) in both the ASN_TEMPLATE and
non-ASN_TEMPLATE code paths of ParseCRL_Extensions.

Testing

Add new CRL certs which have >57 octets CRL number

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@miyazakh miyazakh self-assigned this Mar 5, 2026
@miyazakh miyazakh added the For This Release Release version 5.9.0 label Mar 5, 2026
@miyazakh miyazakh changed the title fix lareger(>57 octets) crlnum fix lareger(>57 octets) CRL number Mar 5, 2026
@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 5, 2026

retest this please

1 similar comment
@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 5, 2026

retest this please

@miyazakh miyazakh assigned wolfSSL-Bot and unassigned miyazakh Mar 5, 2026
@douzzer douzzer added the Conflicts Conflicts with master or staged PRs label Mar 5, 2026
@miyazakh miyazakh force-pushed the fix_larger_crlnum branch from fa8fadd to 5ce86cf Compare March 6, 2026 01:57
@douzzer douzzer added Staged Staged for merge pending final test results and review and removed Conflicts Conflicts with master or staged PRs labels Mar 6, 2026
@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 6, 2026

retest this please

@douzzer douzzer self-requested a review March 6, 2026 04:58
Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

In one test run I saw this anomaly:

[quantum-safe-wolfssl-all-g++-latest-debug] [9 of 55] [a3b8c36cce]
    configure...   real 0m11.855s  user 0m7.757s  sys 0m5.159s
    build...   real 0m10.114s  user 1m50.869s  sys 0m7.683s
    check...FAIL: scripts/crl-gen-openssl.test
   real 0m25.253s  user 0m13.917s  sys 0m5.795s

scripts/crl-gen-openssl.log tail:
Checking RSA CRL: certs/crl/crlRsaOut.pem
expected successful verification for RSA CRL with  certs/client-ca-cert.pem
FAIL scripts/crl-gen-openssl.test (exit status: 1)

check exited with status 2
    scenario started 2026-03-06T04:38:28.290456Z, real elapsed 0m47.403860s
    quantum-safe-wolfssl-all-g++-latest-debug fail_check
    failed config: 'EXTRA_CPPFLAGS=-Werror' '--srcdir' '.' '--disable-jobserver' '--enable-option-checking=fatal' '--enable-all' '--enable-acert' '--enable-dtls13' '--enable-dtls-mtu' '--enable-dtls-frag-ch' '--enable-dtlscid' '--enable-quic' '--with-sys-crypto-policy' '--enable-debug' '--enable-debug-trace-errcodes' '--enable-sp-math-all' '--enable-experimental' '--enable-kyber=yes,original' '--enable-lms' '--enable-xmss' '--enable-dilithium' '--enable-dual-alg-certs' '--disable-qt' 'CC=g++-16' 'CFLAGS=-DTEST_ALWAYS_RUN_TO_END' 'CPPFLAGS=-DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK'
    EXE: 'run_untrusted_task' 'make' 'EXTRA_CFLAGS=-fdiagnostics-color=always -fmax-errors=20' '--quiet' 'WOLFSSL_OPENSSL_TEST=1' '-j' '36' 'check'

A second run didn't repeat the error. I think there might be a stochastic bug here and I want to test more thoroughly.

@douzzer douzzer removed the Staged Staged for merge pending final test results and review label Mar 6, 2026
@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 6, 2026

Sure. I didn't see the error on my side. Used g++-14 instead of g++-16.

./configure EXTRA_CPPFLAGS=-Werror --srcdir . --disable-jobserver --enable-option-checking=fatal --enable-all --enable-acert --enable-dtls13 --enable-dtls-mtu --enable-dtls-frag-ch --enable-dtlscid --enable-quic --with-sys-crypto-policy --enable-debug --enable-debug-trace-errcodes --enable-sp-math-all --enable-experimental --enable-kyber=yes,original --enable-lms --enable-xmss --enable-dilithium --enable-dual-alg-certs --disable-qt CC=g++-14 CFLAGS=-DTEST_ALWAYS_RUN_TO_END 'CPPFLAGS=-DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK'

@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 6, 2026

retest this please

Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

Saw the glitch on scripts/crl-gen-openssl.test in a run without this PR, so this one wasn't to blame for it.

@miyazakh
Copy link
Contributor Author

miyazakh commented Mar 7, 2026

retest this please

@douzzer douzzer merged commit b3f08f3 into wolfSSL:master Mar 7, 2026
450 of 452 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants