certreq: Avoid OOB read when enumerating hashes in OCSP CERTREQ

These certificate requests also contain SHA-1 hashes, which is assumed
in `ike_cert_pre.c::process_certreq()` when enumerating key IDs.

Because the parser allocates a separate chunk for the data and the
enumerator doesn't read beyond that chunk's length after the first
iteration, only lengths between 1 and 19 are problematic (0 doesn't
cause an enumeration because chunk_empty is assigned).

Whether the OOB read then can cause a segmentation fault depends on the
allocator, its alignment rules, and its minimum overhead.  For instance,
with glibc on a typical 64-bit system (8 bytes for pointers and size_t),
the alignment is 16 bytes and the minimum allocated size is 32 bytes,
with typically 24 that are technically available for data, even if only
0 bytes are allocated (as returned by `malloc_usable_size()`).  So with
an allocation between 1 and 19, we can always safely read 20 bytes.

Assuming that other allocators behave similar for small allocations, it
seems unlikely that this causes a crash.

Fixes: 15612b3a4243 ("Add support for IKEv2 OCSP extensions (RFC 4806)")

diff --git a/src/libcharon/encoding/payloads/certreq_payload.c b/src/libcharon/encoding/payloads/certreq_payload.c
index 1b3c8cff245ba10cd86f867ae4027a9f9151bed8..a7d38def2035177d413168c9493e23fade777b70 100644 (file)
--- a/src/libcharon/encoding/payloads/certreq_payload.c
+++ b/src/libcharon/encoding/payloads/certreq_payload.c
@@ -112,12 +112,13 @@ METHOD(payload_t, verify, status_t,
        private_certreq_payload_t *this)
 {
        if (this->type == PLV2_CERTREQ &&
-               this->encoding == ENC_X509_SIGNATURE)
+               (this->encoding == ENC_X509_SIGNATURE ||
+                this->encoding == ENC_OCSP_CONTENT))
        {
                if (this->data.len % HASH_SIZE_SHA1)
                {
-                       DBG1(DBG_ENC, "invalid X509 hash length (%d) in certreq",
-                                this->data.len);
+                       DBG1(DBG_ENC, "invalid hash length (%d) in %N cert request",
+                                this->data.len, cert_encoding_names, this->encoding);
                        return FAILED;
                }
        }