]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
ech: fix off-by-one in hpke_decrypt_encch extensions length bounds check
authorDaniel Cuthbert <daniel@seventysix.io>
Tue, 17 Mar 2026 18:58:33 +0000 (18:58 +0000)
committerEugene Syromiatnikov <esyr@openssl.org>
Fri, 20 Mar 2026 18:09:57 +0000 (19:09 +0100)
The bounds check before reading the two-byte extensions length field uses
extsoffset + 1 instead of extsoffset + 2:

    if ((extsoffset + 1) > clearlen) { goto paderr; }
    extslen = clear[extsoffset] * 256 + clear[extsoffset + 1];

When extsoffset == clearlen - 1 the check passes, but the second read
clear[extsoffset + 1] is clear[clearlen], which is one byte beyond
the decrypted plaintext.  The allocation is OPENSSL_malloc(cipherlen)
where cipherlen = clearlen + AEAD_overhead, so the address is valid,
but the byte is uninitialised after OSSL_HPKE_open returns.

Using Valgrind confirmed an uninitialised-value read at this location
via the full server handshake path:

    hpke_decrypt_encch (ech_internal.c)
    ossl_ech_early_decrypt
    tls_process_client_hello
    state_machine
    SSL_do_handshake

The subsequent ch_len > clearlen check (line 1875) acts as a safety net
and prevents the stale byte from being used further, so the practical
impact is a forced decode error rather than memory disclosure.
Nevertheless, the read itself is incorrect and should be fixed.

Fix: change the guard to extsoffset + 2 so that both bytes
of the extensions length field are confirmed to be within the decrypted
buffer before either is read.

This issue was identified through AI-assisted structural analysis
(RAPTOR) using CodeQL database tooling (AST analysis, control flow
verification, dominator tree analysis) against the OpenSSL master
branch.  The off-by-one was confirmed via AST inspection showing
GT(Add(extsoffset, 1), clearlen) instead of the expected
GT(Add(extsoffset, 2), clearlen).

Found by myself @danielcuthbert and validated
by Benjamin Rodes - Microsoft @bdrodes.

CLA: trivial
Fixes: 6c3edd4f3a8a "Add server-side handling of Encrypted Client Hello"
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
MergeDate: Fri Mar 20 18:10:19 2026
(Merged from https://github.com/openssl/openssl/pull/30472)

ssl/ech/ech_internal.c

index dd0e40fc1bc1d610c65031df6ddfd758dfd33fbb..338c111bfd07c05217f74ca0e648b9fbf32cefdb 100644 (file)
@@ -1864,7 +1864,7 @@ end:
             goto paderr;
         }
         /* odd form of check below just for emphasis */
-        if ((extsoffset + 1) > clearlen) {
+        if ((extsoffset + 2) > clearlen) {
             SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
             goto paderr;
         }