From: Timo Sirainen Date: Fri, 1 May 2026 06:28:33 +0000 (+0000) Subject: [PATCH 3/3] lib-sieve: edit_mail_headers_parse() - Fix info leak via NUL-truncated... X-Git-Tag: archive/raspbian/1%2.4.1+dfsg1-6+rpi1+deb13u7^2~26 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=57af15638a12a4f7e8b9a0a14b0ee74ff9f01585;p=dovecot.git [PATCH 3/3] lib-sieve: edit_mail_headers_parse() - Fix info leak via NUL-truncated header copy i_strndup() stops at the first NUL byte, so embedded NULs caused field->data to be shorter than field->size. This left stale heap data readable past the copied content when callers treat data+body_offset as a string. Adds a test-edit-mail unit test and a sieve testsuite case that both exercise edit_mail_headers_parse() with a NUL byte embedded in a header value. The unit test memcmp-verifies that the bytes after the NUL are preserved; the sieve test triggers an invalid heap read detectable by valgrind under the old code. Gbp-Pq: Name 0003-lib-sieve-edit_mail_headers_parse-Fix-info-leak-via-.patch --- diff --git a/pigeonhole/Makefile.am b/pigeonhole/Makefile.am index 09e67d3..fc49035 100644 --- a/pigeonhole/Makefile.am +++ b/pigeonhole/Makefile.am @@ -169,6 +169,7 @@ test_cases = \ tests/extensions/editheader/utf8.svtest \ tests/extensions/editheader/protected.svtest \ tests/extensions/editheader/errors.svtest \ + tests/extensions/editheader/nul-value.svtest \ tests/extensions/editheader/execute.svtest \ tests/extensions/duplicate/errors.svtest \ tests/extensions/duplicate/execute.svtest \ diff --git a/pigeonhole/src/lib-sieve/util/edit-mail.c b/pigeonhole/src/lib-sieve/util/edit-mail.c index 50ba14f..0a42ca6 100644 --- a/pigeonhole/src/lib-sieve/util/edit-mail.c +++ b/pigeonhole/src/lib-sieve/util/edit-mail.c @@ -553,8 +553,9 @@ edit_mail_header_field_create(struct edit_mail *edmail, const char *field_name, &field->body_offset); /* Copy to new field */ - field->data = i_strndup(str_data(data), str_len(data)); field->size = str_len(data); + field->data = i_malloc(field->size + 1); + memcpy(field->data, str_data(data), field->size); field->virtual_size = (edmail->crlf ? field->size : field->size + lines); field->lines = lines; @@ -831,8 +832,9 @@ static int edit_mail_headers_parse(struct edit_mail *edmail) field->size = str_len(hdr_data); field->virtual_size = field->size + vsize_diff; - field->data = i_strndup(str_data(hdr_data), - field->size); + field->data = i_malloc(field->size + 1); + memcpy(field->data, str_data(hdr_data), + field->size); field->offset = offset; field->lines = lines; diff --git a/pigeonhole/src/lib-sieve/util/test-edit-mail.c b/pigeonhole/src/lib-sieve/util/test-edit-mail.c index 2f834b6..9633a88 100644 --- a/pigeonhole/src/lib-sieve/util/test-edit-mail.c +++ b/pigeonhole/src/lib-sieve/util/test-edit-mail.c @@ -866,6 +866,144 @@ static void test_edit_mail_empty(void) test_end(); } +static void test_edit_mail_nul_in_header(void) +{ + /* Message with a NUL byte embedded in the X-Has-NUL header value. + * sizeof() - 1 gives the true byte count, skipping the C string's + * trailing NUL terminator, while preserving the embedded \x00. */ + static const unsigned char message[] = + "From: sender@example.com\n" + "X-Has-NUL: value\x00" "afterNUL\n" + "Subject: Test\n" + "\n" + "Body\n"; + /* Expected output after deleting the Subject header. */ + static const unsigned char expected[] = + "From: sender@example.com\n" + "X-Has-NUL: value\x00" "afterNUL\n" + "\n" + "Body\n"; + struct istream *input_msg, *input_mail; + buffer_t *buffer; + struct mail_raw *rawmail; + struct edit_mail *edmail; + struct mail *mail; + + test_begin("edit-mail - NUL byte in header value"); + test_edit_mail_init(); + + /* sizeof - 1 excludes the trailing C-string NUL terminator */ + input_msg = i_stream_create_from_data(message, sizeof(message) - 1); + + rawmail = mail_raw_open_stream(test_raw_mail_user, input_msg); + edmail = edit_mail_wrap(rawmail->mail); + + /* Deleting any header triggers edit_mail_headers_parse(), which with + * the old i_strndup() allocated only strlen("X-Has-NUL: value")+1=17 + * bytes for X-Has-NUL's field->data even though field->size=26. + * Streaming then called memcpy(dst, field->data, 26), reading 9 bytes + * past the end of the allocation and producing garbage in place of + * "afterNUL\n". */ + edit_mail_header_delete(edmail, "Subject", 0); + + mail = edit_mail_get_mail(edmail); + + if (mail_get_stream(mail, NULL, NULL, &input_mail) < 0) { + i_fatal("Failed to open mail stream: %s", + mailbox_get_last_internal_error(mail->box, NULL)); + } + + buffer = buffer_create_dynamic(default_pool, 128); + + /* normal */ + + i_stream_seek(input_mail, 0); + test_stream_data(input_mail, buffer); + + test_out("nul in header", + buffer->used == sizeof(expected) - 1 && + memcmp(buffer->data, expected, sizeof(expected) - 1) == 0); + + /* slow (byte-by-byte) */ + + i_stream_seek(input_mail, 0); + buffer_set_used_size(buffer, 0); + test_stream_data_slow(input_mail, buffer); + + test_out("nul in header, slow", + buffer->used == sizeof(expected) - 1 && + memcmp(buffer->data, expected, sizeof(expected) - 1) == 0); + + /* clean up */ + + buffer_free(&buffer); + edit_mail_unwrap(&edmail); + mail_raw_close(&rawmail); + i_stream_unref(&input_msg); + test_edit_mail_deinit(); + test_end(); +} + +static void test_edit_mail_empty2(void) +{ + struct istream *input_msg, *input_mail; + buffer_t *buffer; + struct mail_raw *rawmail; + struct edit_mail *edmail; + struct mail *mail; + const char *value; + + test_begin("edit-mail - empty message (delete, add)"); + test_edit_mail_init(); + + /* Compose the message */ + + input_msg = i_stream_create_from_data("", 0); + + rawmail = mail_raw_open_stream(test_raw_mail_user, input_msg); + + edmail = edit_mail_wrap(rawmail->mail); + + /* Delete header */ + + edit_mail_header_delete(edmail, "X-B", 0); + + /* Add header */ + + edit_mail_header_add(edmail, "X-B", "Frop", TRUE); + mail = edit_mail_get_mail(edmail); + + /* Prepare tests */ + + if (mail_get_stream(mail, NULL, NULL, &input_mail) < 0) { + i_fatal("Failed to open mail stream: %s", + mailbox_get_last_error(mail->box, NULL)); + } + + buffer = buffer_create_dynamic(default_pool, 1024); + + /* Evaluate modified header */ + + test_assert(mail_get_first_header_utf8(mail, "X-B", &value) > 0 && + strcmp(value, "Frop") == 0); + + /* Added */ + + i_stream_seek(input_mail, 0); + buffer_set_used_size(buffer, 0); + + test_stream_data(input_mail, buffer); + + /* Clean up */ + + buffer_free(&buffer); + edit_mail_unwrap(&edmail); + mail_raw_close(&rawmail); + i_stream_unref(&input_msg); + test_edit_mail_deinit(); + test_end(); +} + int main(int argc, char *argv[]) { static void (*test_functions[])(void) = { @@ -873,6 +1011,8 @@ int main(int argc, char *argv[]) test_edit_mail_big_header, test_edit_mail_small_buffer, test_edit_mail_empty, + test_edit_mail_empty2, + test_edit_mail_nul_in_header, NULL }; const enum master_service_flags service_flags = diff --git a/pigeonhole/tests/extensions/editheader/nul-value.svtest b/pigeonhole/tests/extensions/editheader/nul-value.svtest new file mode 100644 index 0000000..9034e6c --- /dev/null +++ b/pigeonhole/tests/extensions/editheader/nul-value.svtest @@ -0,0 +1,46 @@ +require "vnd.dovecot.testsuite"; +require "encoded-character"; +require "variables"; +require "fileinto"; +require "mailbox"; + +require "editheader"; + +/* + * Regression test: edit_mail_headers_parse() used i_strndup() to copy the + * raw header data, which stopped at the first NUL byte and allocated a buffer + * shorter than field->size. When the modified message was subsequently + * streamed (e.g. fileinto), memcpy() read field->size bytes out of that + * under-sized buffer, causing an invalid heap read detectable by valgrind. + */ + +/* Construct a raw message that has a NUL byte inside a header value. + * ${hex:00} = NUL. field->size covers the full header line including + * bytes after the NUL; the old i_strndup() only allocated up to the NUL. */ +test_set "message" text: +From: sender@example.com +X-Has-NUL: value${hex:00}afterNUL +Subject: Test + +Body +. +; + +test "editheader - NUL byte in header value" { + /* deleteheader calls edit_mail_headers_parse(), which allocates + * field->data for every header including X-Has-NUL. With the old + * i_strndup() code, X-Has-NUL's allocation is only strlen("value")+1 + * bytes, but field->size covers the full extent including bytes after + * the NUL. deleteheader also sets modified=TRUE. */ + deleteheader "Subject"; + + fileinto :create "nul-test"; + + /* Streaming iterates over all remaining parsed headers including + * X-Has-NUL, reading field->size bytes from field->data. With the + * old under-sized allocation this is an invalid heap read that + * valgrind detects. */ + if not test_result_execute { + test_fail "failed to execute result"; + } +}