From 65ab33fa54e34fba69d793735b7df3d383d1ff78 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Fri, 17 Apr 2026 18:21:36 +0200 Subject: buffers: add more checks to DTLS reassembly Previously, gnutls didn't check that DTLS fragments claimed a consistent message_length value. Additionally, a crucial array size check was missing, enabling an attacker to cause a heap overwrite. The updated version rejects fragments with mismatching length and adds a missing boundary check. Reported-by: Haruto Kimura (Stella) Reported-by: Oscar Reparaz Reported-by: Zou Dikai Fixes: #1816 Fixes: #1838 Fixes: #1839 Fixes: CVE-2026-33846 Fixes: GNUTLS-SA-2026-04-29-1 CVSS: 7.4 High CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:H CVSS: 7.5 High CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H Signed-off-by: Alexander Sosedkin --- lib/buffers.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) From 9deffca528c23bbb218f5ec3bd4bb1bf4cbd1fc0 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Fri, 17 Apr 2026 17:49:31 +0200 Subject: [PATCH] buffers: shorten merge_handshake_packet using recv_buf I had vague concerns about thread-safety of this, but then this pattern already exists within the file. Signed-off-by: Alexander Sosedkin --- lib/buffers.c | 52 +++++++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) From 4f94e5cfe1f252a431e41642b0752e7e0daf43b9 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Fri, 20 Mar 2026 16:09:40 +0100 Subject: [PATCH] tests/mini-dtls-fragments: implement a basic DTLS test Signed-off-by: Alexander Sosedkin --- tests/Makefile.am | 7 +- tests/mini-dtls-fragments.c | 208 ++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 tests/mini-dtls-fragments.c Index: gnutls-3.8.10/tests/Makefile.am =================================================================== --- gnutls-3.8.10.orig/tests/Makefile.am +++ gnutls-3.8.10/tests/Makefile.am @@ -241,7 +241,8 @@ ctests += mini-record-2 simple gnutls_hm x509cert-dntypes id-on-xmppAddr tls13-compat-mode ciphersuite-name \ x509-upnconstraint xts-key-check cipher-padding pkcs7-verify-double-free \ fips-rsa-sizes tls12-rehandshake-ticket pathbuf tls-force-ems \ - psk-importer privkey-derive dh-compute2 ecdh-compute2 + psk-importer privkey-derive dh-compute2 ecdh-compute2 \ + mini-dtls-fragments ctests += tls-channel-binding @@ -500,6 +501,10 @@ pathbuf_CPPFLAGS = $(AM_CPPFLAGS) \ -I$(top_srcdir)/gl \ -I$(top_builddir)/gl +mini_dtls_fragments_CPPFLAGS = $(AM_CPPFLAGS) \ + -I$(top_srcdir)/gl \ + -I$(top_builddir)/gl + if ENABLE_PKCS11 if !WINDOWS ctests += tls13/post-handshake-with-cert-pkcs11 pkcs11/tls-neg-pkcs11-no-key \ Index: gnutls-3.8.10/tests/mini-dtls-fragments.c =================================================================== --- /dev/null +++ gnutls-3.8.10/tests/mini-dtls-fragments.c @@ -0,0 +1,475 @@ +/* + * Copyright (C) 2026 Red Hat, Inc. + * + * Author: Alexander Sosedkin + * + * This file is part of GnuTLS. + * + * GnuTLS is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * GnuTLS is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include + +#if defined(_WIN32) + +int main(void) +{ + exit(77); +} + +#else + +#include +#include +#include +#include +#include +#include +#include +#include "cert-common.h" +#include "utils.h" + +#include "attribute.h" + +static void server_log_func(int level, const char *str) +{ + fprintf(stderr, "server|<%d>| %s", level, str); +} + +static void client_log_func(int level, const char *str) +{ + fprintf(stderr, "client|<%d>| %s", level, str); +} + +#define QUEUE_SIZE 1024 +#define PACKET_SIZE 2048 + +typedef struct { + uint8_t buf[PACKET_SIZE]; + size_t len; +} packet_t; +typedef struct { + packet_t packets[QUEUE_SIZE]; + size_t head; + size_t tail; +} queue_t; + +static queue_t c2s, s2c; + +static int queue_put(queue_t *q, const void *buf, size_t len) +{ + assert(len <= PACKET_SIZE); + memcpy(q->packets[q->tail].buf, buf, len); + q->packets[q->tail].len = len; + q->tail++; + q->tail %= QUEUE_SIZE; + assert(q->tail != q->head); + return len; +} + +static ssize_t queue_get(queue_t *q, gnutls_session_t s, void *buf, size_t len) +{ + if (q->head == q->tail) { + gnutls_transport_set_errno(s, EAGAIN); + return -1; + } + size_t n = q->packets[q->head].len; + memcpy(buf, q->packets[q->head].buf, n); + q->head++; + q->head %= QUEUE_SIZE; + return n; +} + +static void queue_reset(queue_t *q) +{ + q->head = q->tail = 0; +} + +static int pull_timeout(gnutls_transport_ptr_t tr, unsigned ms) +{ + return 1; +} + +static int c2s_pull_timeout_once(gnutls_transport_ptr_t tr, unsigned ms) +{ + return c2s.head != c2s.tail ? 1 : 0; +} + +static ssize_t server_pull(gnutls_transport_ptr_t tr, void *b, size_t l) +{ + return queue_get(&c2s, (gnutls_session_t)tr, b, l); +} + +static ssize_t client_pull(gnutls_transport_ptr_t tr, void *b, size_t l) +{ + return queue_get(&s2c, (gnutls_session_t)tr, b, l); +} + +static ssize_t server_push(gnutls_transport_ptr_t tr, const void *b, size_t l) +{ + return queue_put(&s2c, b, l); +} + +static ssize_t client_push_normal(gnutls_transport_ptr_t tr, const void *b, + size_t l) +{ + return queue_put(&c2s, b, l); +} + +static void write_u16(uint8_t *p, uint16_t val) +{ + p[0] = val >> 8; + p[1] = val & 0xff; +} + +static void write_u24(uint8_t *p, uint32_t val) +{ + p[0] = (val >> 16) & 0xff; + p[1] = (val >> 8) & 0xff; + p[2] = val & 0xff; +} + +static void write_u48(uint8_t *p, uint64_t seq) +{ + int i; + for (i = 5; i >= 0; i--) { + p[i] = seq & 0xff; + seq >>= 8; + } +} + +static uint64_t read_u48(const uint8_t *p) +{ + uint64_t seq = 0; + int i; + for (i = 5; i >= 0; i--) { + seq <<= 8; + seq |= p[i]; + } + return seq; +} + +static void test(gnutls_push_func client_push, bool expect_success) +{ + gnutls_session_t client, server; + gnutls_certificate_credentials_t ccred, scred; + int cr = 0, sr = 0; + bool cdone = false, sdone = false; + + if (debug) + gnutls_global_set_log_level(4711); + + gnutls_certificate_allocate_credentials(&scred); + gnutls_certificate_set_x509_key_mem(scred, &server_cert, &server_key, + GNUTLS_X509_FMT_PEM); + gnutls_certificate_allocate_credentials(&ccred); + + gnutls_init(&server, GNUTLS_SERVER | GNUTLS_DATAGRAM); + gnutls_init(&client, GNUTLS_CLIENT | GNUTLS_DATAGRAM); + + gnutls_priority_set_direct(server, "NORMAL:-VERS-ALL:+VERS-DTLS1.2", + NULL); + gnutls_priority_set_direct(client, "NORMAL:-VERS-ALL:+VERS-DTLS1.2", + NULL); + + gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, scred); + gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, ccred); + + gnutls_dtls_set_timeouts(client, get_dtls_retransmit_timeout(), + get_timeout()); + gnutls_dtls_set_timeouts(server, get_dtls_retransmit_timeout(), + get_timeout()); + + gnutls_transport_set_ptr(client, client); + gnutls_transport_set_push_function(client, client_push); + gnutls_transport_set_pull_function(client, client_pull); + gnutls_transport_set_pull_timeout_function(client, pull_timeout); + + gnutls_transport_set_ptr(server, server); + gnutls_transport_set_push_function(server, server_push); + gnutls_transport_set_pull_function(server, server_pull); + gnutls_transport_set_pull_timeout_function(server, pull_timeout); + + while (!cdone || !sdone) { + gnutls_global_set_log_function(client_log_func); + if (!cdone) + cr = gnutls_handshake(client); + if (!cr || gnutls_error_is_fatal(cr)) + cdone = true; + + gnutls_global_set_log_function(server_log_func); + if (!sdone) + sr = gnutls_handshake(server); + if (!sr || gnutls_error_is_fatal(sr)) + sdone = true; + + if (c2s.head == c2s.tail && s2c.head == s2c.tail) + break; /* speed the test up */ + } + + if (expect_success) { + if (cr) + fail("client: %s\n", gnutls_strerror(cr)); + if (sr) + fail("server: %s\n", gnutls_strerror(sr)); + + } else { + if (cr == 0 && sr == 0) + fail("handshake unexpectedly succeeded: %s / %s\n", + gnutls_strerror(cr), gnutls_strerror(sr)); + } + + success("OK\n"); + + queue_reset(&c2s); + queue_reset(&s2c); + + gnutls_deinit(client); + gnutls_deinit(server); + gnutls_certificate_free_credentials(ccred); + gnutls_certificate_free_credentials(scred); +} + +static void test_malicious1816(void) +{ + /* dgram1: msg_len=50, frag_offset=25, frag_len=25 */ + static const uint8_t dgram1_hdr[] = { + 0x16, /* type: handshake */ + 0xfe, 0xfd, /* version: DTLS 1.2 */ + 0x00, 0x00, /* epoch: 0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* seq: 0 */ + 0x00, 0x25, /* record_length: 37 */ + 0x01, /* msg_type: ClientHello */ + 0x00, 0x00, 0x32, /* msg_length: 50 */ + 0x00, 0x00, /* msg_seq: 0 */ + 0x00, 0x00, 0x19, /* frag_offset: 25 */ + 0x00, 0x00, 0x19, /* frag_length: 25 */ + }; + /* dgram2: msg_len=3000, frag_offset=0, frag_len=48 */ + static const uint8_t dgram2_hdr[] = { + 0x16, /* type: handshake */ + 0xfe, 0xfd, /* version: DTLS 1.2 */ + 0x00, 0x00, /* epoch: 0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, /* seq: 1 */ + 0x00, 0x3c, /* record_length: 60 */ + 0x01, /* msg_type: ClientHello */ + 0x00, 0x0b, 0xb8, /* msg_length: 3000 */ + 0x00, 0x00, /* msg_seq: 0 */ + 0x00, 0x00, 0x00, /* frag_offset: 0 */ + 0x00, 0x00, 0x30, /* frag_length: 48 */ + }; + /* dgram3: msg_len=3000, frag_offset=40, frag_len=1475 */ + static const uint8_t dgram3_hdr[] = { + 0x16, /* type: handshake */ + 0xfe, 0xfd, /* version: DTLS 1.2 */ + 0x00, 0x00, /* epoch: 0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, /* seq: 2 */ + 0x05, 0xcf, /* record_length: 1487 */ + 0x01, /* msg_type: ClientHello */ + 0x00, 0x0b, 0xb8, /* msg_length: 3000 */ + 0x00, 0x00, /* msg_seq: 0 */ + 0x00, 0x00, 0x28, /* frag_offset: 40 */ + 0x00, 0x05, 0xc3, /* frag_length: 1475 */ + }; + /* dgram4: msg_len=3000, frag_offset=1500, frag_len=1475 */ + static const uint8_t dgram4_hdr[] = { + 0x16, /* type: handshake */ + 0xfe, 0xfd, /* version: DTLS 1.2 */ + 0x00, 0x00, /* epoch: 0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, /* seq: 3 */ + 0x05, 0xcf, /* record_length: 1487 */ + 0x01, /* msg_type: ClientHello */ + 0x00, 0x0b, 0xb8, /* msg_length: 3000 */ + 0x00, 0x00, /* msg_seq: 0 */ + 0x00, 0x05, 0xdc, /* frag_offset: 1500 */ + 0x00, 0x05, 0xc3, /* frag_length: 1475 */ + }; + gnutls_session_t server; + gnutls_certificate_credentials_t scred; + uint8_t dgram[1500]; + int sr; + + if (debug) + gnutls_global_set_log_level(4711); + + gnutls_certificate_allocate_credentials(&scred); + gnutls_certificate_set_x509_key_mem(scred, &server_cert, &server_key, + GNUTLS_X509_FMT_PEM); + + gnutls_init(&server, GNUTLS_SERVER | GNUTLS_DATAGRAM); + gnutls_priority_set_direct(server, "NORMAL:+VERS-DTLS1.2", NULL); + gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, scred); + + gnutls_dtls_set_timeouts(server, get_dtls_retransmit_timeout(), + get_timeout()); + + gnutls_transport_set_ptr(server, server); + gnutls_transport_set_push_function(server, server_push); + gnutls_transport_set_pull_function(server, server_pull); + gnutls_transport_set_pull_timeout_function(server, + c2s_pull_timeout_once); + + memset(dgram, 0, sizeof(dgram)); + memcpy(dgram, dgram1_hdr, 25); + queue_put(&c2s, dgram, 25 + 25); + + memset(dgram, 0, sizeof(dgram)); + memcpy(dgram, dgram2_hdr, 25); + queue_put(&c2s, dgram, 25 + 48); + + memset(dgram, 0, sizeof(dgram)); + memcpy(dgram, dgram3_hdr, 25); + queue_put(&c2s, dgram, 25 + 1475); + + memset(dgram, 0, sizeof(dgram)); + memcpy(dgram, dgram4_hdr, 25); + queue_put(&c2s, dgram, 25 + 1475); + + gnutls_global_set_log_function(server_log_func); + do { + sr = gnutls_handshake(server); /* invalid write if vulnerable */ + } while (c2s.head != c2s.tail && !gnutls_error_is_fatal(sr)); + if (sr != GNUTLS_E_UNEXPECTED_PACKET_LENGTH) + fail("server: expected GNUTLS_E_UNEXPECTED_PACKET_LENGTH, " + "got: %s\n", + gnutls_strerror(sr)); + + success("OK\n"); + + queue_reset(&c2s); + queue_reset(&s2c); + + gnutls_deinit(server); + gnutls_certificate_free_credentials(scred); +} + +static ssize_t queue_put_renumbered(queue_t *q, const uint8_t *data, size_t l, + int delta_n) +{ + if (delta_n == 0 || l < 13 || data[3] != 0 || data[4] != 0) + return queue_put(&c2s, data, l); + + uint8_t *p = malloc(l); + assert(p); + memcpy(p, data, l); + write_u48(p + 5, read_u48(p + 5) + delta_n); + ssize_t ret = queue_put(q, p, l); + free(p); + return ret; +} + +static void split_client_hello(const uint8_t *data, size_t len, uint8_t **frag1, + size_t *frag1_len, uint8_t **frag2, + size_t *frag2_len) +{ + size_t body_size = len - 25; + *frag1_len = 13 + 12 + 1; + *frag2_len = 13 + 12 + (body_size - 1); + + *frag1 = malloc(13 + 12 + 1); + assert(*frag1); + *frag2 = malloc(13 + 12 + body_size - 1); + assert(*frag2); + + /* first fragment: record header + handshake header + first body byte */ + memcpy(*frag1, data, 13); /* record header */ + write_u16(*frag1 + 11, 12 + 1); /* record length */ + memcpy(*frag1 + 13, data + 13, 12); /* handshake header */ + write_u24(*frag1 + 19, 0); /* fragment_offset = 0 */ + write_u24(*frag1 + 22, 1); /* fragment_length = 1 */ + (*frag1)[25] = data[25]; /* first body byte */ + + /* second fragment: record header + handshake header + remaining body */ + memcpy(*frag2, data, 13); /* record header */ + write_u16(*frag2 + 11, *frag2_len - 13); /* record length */ + write_u48(*frag2 + 5, read_u48(*frag2 + 5) + 1); /* sequence number */ + memcpy(*frag2 + 13, data + 13, 12); /* handshake header */ + write_u24(*frag2 + 19, 1); /* fragment_offset = 1 */ + write_u24(*frag2 + 22, body_size - 1); /* shortened fragment_length */ + memcpy(*frag2 + 25, data + 26, body_size - 1); /* remaining body */ +} + +static ssize_t client_push_split_hello(gnutls_transport_ptr_t tr, const void *b, + size_t l) +{ + static int seq_offset = 0; /* for renumbering follow-up epoch0 ones */ + + const uint8_t *data = (const uint8_t *)b; + uint8_t *frag1, *frag2; + size_t frag1_len, frag2_len; + + /* Pass through anything that isn't an epoch0 ClientHello with body */ + if (l < 13 + 12 + 1 || /* too short for DTLS record header */ + data[0] != 22 || /* not a handshake record */ + data[3] != 0 || data[4] != 0 || /* not epoch 0 */ + data[13] != 1) /* not ClientHello */ + return queue_put_renumbered(&c2s, b, l, seq_offset); + + /* epoch0 Client Hello: special treatment of splitting into fragments */ + split_client_hello(data, l, &frag1, &frag1_len, &frag2, &frag2_len); + queue_put(&c2s, frag1, frag1_len); + queue_put(&c2s, frag2, frag2_len); + free(frag1); + free(frag2); + seq_offset++; + return l; +} + +static ssize_t client_push_split_hello_bad_seq(gnutls_transport_ptr_t tr, + const void *b, size_t l) +{ + /* gnutls wasn't matching on message_seq on merging, see #1839 */ + static int seq_offset = 0; /* for renumbering follow-up epoch0 ones */ + + const uint8_t *data = (const uint8_t *)b; + uint8_t *frag1, *frag2; + size_t frag1_len, frag2_len; + + /* Pass through anything that isn't an epoch0 ClientHello with body */ + if (l < 13 + 12 + 1 || /* too short for DTLS record header */ + data[0] != 22 || /* not a handshake record */ + data[3] != 0 || data[4] != 0 || /* not epoch 0 */ + data[13] != 1) /* not ClientHello */ + return queue_put_renumbered(&c2s, b, l, seq_offset); + + /* epoch0 Client Hello: special treatment of splitting into fragments */ + split_client_hello(data, l, &frag1, &frag1_len, &frag2, &frag2_len); + queue_put(&c2s, frag1, frag1_len); + frag2[18]++; /* WRONG, message_seq mismatch must be rejected, #1839 */ + queue_put(&c2s, frag2, frag2_len); + free(frag1); + free(frag2); + seq_offset++; + return l; +} + +void doit(void) +{ + global_init(); + test(client_push_normal, true); + success("malicious reassembly bug exploitation (#1816):\n"); + test_malicious1816(); + success("split client hello smoke-test\n"); + test(client_push_split_hello, true); + success("split client hello smoke-test and mangle sequence number\n"); + test(client_push_split_hello_bad_seq, false); + gnutls_global_deinit(); +} + +#endif /* _WIN32 */ Index: gnutls-3.8.10/lib/buffers.c =================================================================== --- gnutls-3.8.10.orig/lib/buffers.c +++ gnutls-3.8.10/lib/buffers.c @@ -967,9 +967,12 @@ static int merge_handshake_packet(gnutls int exists = 0, i, pos = 0; int ret; + handshake_buffer_st *recv_buf = + session->internals.handshake_recv_buffer; + for (i = 0; i < session->internals.handshake_recv_buffer_size; i++) { - if (session->internals.handshake_recv_buffer[i].htype == - hsk->htype) { + if (recv_buf[i].htype == hsk->htype && + recv_buf[i].sequence == hsk->sequence) { exists = 1; pos = i; break; @@ -1005,44 +1008,44 @@ static int merge_handshake_packet(gnutls _gnutls_write_uint24(0, &hsk->header[6]); _gnutls_write_uint24(hsk->length, &hsk->header[9]); - _gnutls_handshake_buffer_move( - &session->internals.handshake_recv_buffer[pos], hsk); + _gnutls_handshake_buffer_move(&recv_buf[pos], hsk); } else { - if (hsk->start_offset < - session->internals.handshake_recv_buffer[pos] - .start_offset && - hsk->end_offset + 1 >= - session->internals.handshake_recv_buffer[pos] - .start_offset) { - memcpy(&session->internals.handshake_recv_buffer[pos] - .data.data[hsk->start_offset], + if (hsk->length != recv_buf[pos].length) { + /* inconsistent across fragments */ + _gnutls_handshake_buffer_clear(hsk); + return gnutls_assert_val( + GNUTLS_E_UNEXPECTED_PACKET_LENGTH); + } + /* start_offset + data.length <= hsk->length <= max_length */ + if (hsk->length < hsk->start_offset + hsk->data.length) { + /* impossible claims, overflow requested */ + _gnutls_handshake_buffer_clear(hsk); + return gnutls_assert_val( + GNUTLS_E_UNEXPECTED_PACKET_LENGTH); + } + if (hsk->length > recv_buf[pos].data.max_length) { + /* we don't have this much allocated, overflow guard */ + _gnutls_handshake_buffer_clear(hsk); + return gnutls_assert_val( + GNUTLS_E_UNEXPECTED_PACKET_LENGTH); + } + + if (hsk->start_offset < recv_buf[pos].start_offset && + hsk->end_offset + 1 >= recv_buf[pos].start_offset) { + memcpy(&recv_buf[pos].data.data[hsk->start_offset], hsk->data.data, hsk->data.length); - session->internals.handshake_recv_buffer[pos] - .start_offset = hsk->start_offset; - session->internals.handshake_recv_buffer[pos] - .end_offset = MIN( - hsk->end_offset, - session->internals.handshake_recv_buffer[pos] - .end_offset); - } else if (hsk->end_offset > - session->internals.handshake_recv_buffer[pos] - .end_offset && - hsk->start_offset <= - session->internals.handshake_recv_buffer[pos] - .end_offset + - 1) { - memcpy(&session->internals.handshake_recv_buffer[pos] - .data.data[hsk->start_offset], + recv_buf[pos].start_offset = hsk->start_offset; + recv_buf[pos].end_offset = + MIN(hsk->end_offset, recv_buf[pos].end_offset); + } else if (hsk->end_offset > recv_buf[pos].end_offset && + hsk->start_offset <= recv_buf[pos].end_offset + 1) { + memcpy(&recv_buf[pos].data.data[hsk->start_offset], hsk->data.data, hsk->data.length); - session->internals.handshake_recv_buffer[pos] - .end_offset = hsk->end_offset; - session->internals.handshake_recv_buffer[pos] - .start_offset = MIN( - hsk->start_offset, - session->internals.handshake_recv_buffer[pos] - .start_offset); + recv_buf[pos].end_offset = hsk->end_offset; + recv_buf[pos].start_offset = MIN( + hsk->start_offset, recv_buf[pos].start_offset); } _gnutls_handshake_buffer_clear(hsk); }