From: Marco Bettini Date: Wed, 8 Apr 2026 10:14:08 +0000 (+0000) Subject: [PATCH 4/4] lib-mail: o_stream_dot_sendv() - Do not send unguarded '.' after bare... X-Git-Tag: archive/raspbian/1%2.4.1+dfsg1-6+rpi1+deb13u7^2~65 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=d6e00ee311705d04218c128b320cca8a7e705f3d;p=dovecot.git [PATCH 4/4] lib-mail: o_stream_dot_sendv() - Do not send unguarded '.' after bare '\r' Gbp-Pq: Name 0004-lib-mail-o_stream_dot_sendv-Do-not-send-unguarded-.-.patch --- diff --git a/src/lib-mail/ostream-dot.c b/src/lib-mail/ostream-dot.c index 81930a0..51c8bcf 100644 --- a/src/lib-mail/ostream-dot.c +++ b/src/lib-mail/ostream-dot.c @@ -71,10 +71,12 @@ o_stream_dot_close(struct iostream_private *stream, bool close_parent) o_stream_close(dstream->ostream.parent); } +#define ADD_MAX 3 enum o_stream_dot_sendv_add { ADD_NONE, ADD_CR, ADD_DOT, + ADD_LF_DOT, }; static ssize_t @@ -111,14 +113,14 @@ o_stream_dot_sendv(struct ostream_private *stream, p = data; pend = CONST_PTR_OFFSET(data, size); - for (; p < pend && ((size_t)(p - data) + 2) <= max_bytes; p++) { + for (; p < pend && ((size_t)(p - data) + ADD_MAX) <= max_bytes; p++) { enum o_stream_dot_sendv_add add = ADD_NONE; size = pend - p; switch (dstream->state) { /* none */ case STREAM_STATE_NONE: { - size_t maxlen = I_MIN(size, max_bytes - ((size_t)(p - data) + 2)); + size_t maxlen = I_MIN(size, max_bytes - ((size_t)(p - data) + ADD_MAX)); p = CONST_PTR_OFFSET(p, i_memcspn(p, maxlen, "\r\n", 2)); i_assert(p <= pend); if (p == pend) { @@ -145,6 +147,9 @@ o_stream_dot_sendv(struct ostream_private *stream, case '\n': dstream->state = STREAM_STATE_CRLF; break; + case '.': + add = ADD_LF_DOT; + /* fall through */ default: dstream->state = STREAM_STATE_NONE; break; @@ -186,9 +191,31 @@ o_stream_dot_sendv(struct ostream_private *stream, max_bytes -= chunk; sent += chunk; } - /* insert byte (substitute one with pair) */ data++; + /* Apply the modifications required according to RFC 5321 to the + stream : + + ADD_DOT - Apply standard SMTP dot-stuffing + + ADD_CR - Convert invalid lone LFs to CRLF to comply with + DATA line termination requirements, and avoid + potentially emitting bare "\n." sequences (same + threat as below for "\r." sequences). + + ADD_LF_DOT - Guard against invalid "\r." sequences. + A downstream MTA that incorrectly handles bare CRs + as line terminators could interpret a '.' at the + start of the next line as end-of-DATA. This would + allow any following bytes to be processed as SMTP + commands. + + ADD_CR and ADD_LF_DOT make the istream-dot round-trip lossy, + as these introduce bytes not present in the original input. + We accept this tradeoff, since preserving exact byte fidelity + for malformed input is less important than eliminating an + injection vector. */ + switch(add) { case ADD_DOT: iovn.iov_base = ".."; @@ -198,12 +225,17 @@ o_stream_dot_sendv(struct ostream_private *stream, iovn.iov_base = "\r\n"; iovn.iov_len = 2; break; + case ADD_LF_DOT: + iovn.iov_base = "\n.."; + iovn.iov_len = 3; + break; default: i_unreached(); } array_push_back(&iov_arr, &iovn); - i_assert(max_bytes >= iovn.iov_len); + i_assert(iovn.iov_len <= ADD_MAX); + i_assert(iovn.iov_len <= max_bytes); max_bytes -= iovn.iov_len; added += iovn.iov_len - 1; sent++; diff --git a/src/lib-mail/test-ostream-dot.c b/src/lib-mail/test-ostream-dot.c index 6a1813d..4e1b297 100644 --- a/src/lib-mail/test-ostream-dot.c +++ b/src/lib-mail/test-ostream-dot.c @@ -57,6 +57,9 @@ static void test_ostream_dot(void) { "foo\n.\n", "foo\r\n..\r\n.\r\n" }, { ".foo\r\n.\r\nfoo\r\n", "..foo\r\n..\r\nfoo\r\n.\r\n" }, { ".foo\n.\nfoo\n", "..foo\r\n..\r\nfoo\r\n.\r\n" }, + { ".", "..\r\n.\r\n" }, + { "\r.", "\r\n..\r\n.\r\n" }, + { "\r\r.", "\r\r\n..\r\n.\r\n" }, { "\r\n", "\r\n.\r\n" }, { "\n", "\r\n.\r\n" }, { "", "\r\n.\r\n" }, @@ -92,7 +95,7 @@ static void test_ostream_dot_parent_almost_full(void) test_end(); } -static void test_ostream_dot_parent_exact_fit(void) +static void test_ostream_dot_parent_max_bytes_boundary(void) { buffer_t *output_data; struct ostream *test_output, *output; @@ -100,8 +103,8 @@ static void test_ostream_dot_parent_exact_fit(void) test_begin("dot ostream parent exact fit"); output_data = t_buffer_create(1024); - test_output = test_ostream_create_nonblocking(output_data, 2); - test_ostream_set_max_output_size(test_output, 2); + test_output = test_ostream_create_nonblocking(output_data, 3); + test_ostream_set_max_output_size(test_output, 3); output = o_stream_create_dot(test_output, FALSE); ret = o_stream_send(output, ".", 1); @@ -114,12 +117,72 @@ static void test_ostream_dot_parent_exact_fit(void) test_end(); } +/* STATE_CR must persist across sendv calls so that a bare CR ending one send + followed by '.' starting the next still triggers ADD_LF_DOT. + A stateless per-call implementation would emit the '.' unguarded and let a + downstream MTA that treats bare CR as EOL interpret it as end-of-DATA. */ +static void test_ostream_dot_cr_across_sendv(void) +{ + buffer_t *output_data; + struct ostream *test_output, *output; + ssize_t ret; + const char *expected = "\r\n..\r\n.\r\n"; + + test_begin("dot ostream CR across sendv calls"); + output_data = t_buffer_create(1024); + test_output = o_stream_create_buffer(output_data); + + output = o_stream_create_dot(test_output, FALSE); + ret = o_stream_send(output, "\r", 1); + test_assert(ret == 1); + ret = o_stream_send(output, ".", 1); + test_assert(ret == 1); + test_assert(o_stream_finish(output) > 0); + + o_stream_unref(&output); + o_stream_unref(&test_output); + + test_assert_ucmp(str_len(output_data), ==, strlen(expected)); + test_assert_memcmp(str_c(output_data), str_len(output_data), + expected, strlen(expected)); + test_end(); +} + +/* Off-by-one regression guard for the bare-CR + '.' injection. + Input "\r." places '.' at offset 1; the ADD_LF_DOT pair injects 3 bytes + ("\n.."). The loop guard check becomes (1 + 3) <= max_bytes, so max_bytes==4 + is the exact-fit case. */ +static void test_ostream_dot_cr_dot_exact_fit(void) +{ + buffer_t *output_data; + struct ostream *test_output, *output; + ssize_t ret; + + test_begin("dot ostream CR+dot exact fit"); + output_data = t_buffer_create(1024); + test_output = test_ostream_create_nonblocking(output_data, 4); + test_ostream_set_max_output_size(test_output, 4); + + output = o_stream_create_dot(test_output, FALSE); + ret = o_stream_send(output, "\r.", 2); + test_assert(ret == 2); + test_assert_ucmp(output_data->used, ==, 4); + test_assert_memcmp(output_data->data, output_data->used, + "\r\n..", 4); + + o_stream_unref(&output); + o_stream_unref(&test_output); + test_end(); +} + int main(void) { static void (*const test_functions[])(void) = { test_ostream_dot, test_ostream_dot_parent_almost_full, - test_ostream_dot_parent_exact_fit, + test_ostream_dot_parent_max_bytes_boundary, + test_ostream_dot_cr_across_sendv, + test_ostream_dot_cr_dot_exact_fit, NULL }; return test_run(test_functions);