diff options
| -rw-r--r-- | doc/doc-txt/ChangeLog | 11 | ||||
| -rw-r--r-- | src/src/smtp_in.c | 30 | ||||
| -rw-r--r-- | test/stderr/0435 | 4 | ||||
| -rw-r--r-- | test/stdout/0435 | 2 |
4 files changed, 29 insertions, 18 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 6fb440203..eb0e1a346 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -149,6 +149,17 @@ JH/28 Ensure that variables possibly set during message acceptance are marked message files. Do the same for the SMTP per-message loop, for certain variables indirectly set in ACL operations. +JH/29 Bug 2250: Fix a longstanding bug in heavily-pipelined SMTP input (such + as a multi-recipient message from a mailinglist manager). The coding had + an arbitrary cutoff number of characters while checking for more input; + enforced by writing a NUL into the buffer. This corrupted long / fast + input. The problem was exposed more widely when more pipelineing of SMTP + responses was introduced, and one Exim system was feeding another. + The symptom is log complaints of SMTP syntax error (NUL chars) on the + receiving system, and refused recipients seen by the sending system + (propating to people being dropped from mailing lists). + Discovered and pinpointed by David Carter. + Exim version 4.90 ----------------- diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index c9fe79220..2d213305e 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -359,16 +359,13 @@ rc = smtp_getc(GETC_BUFFER_UNLIMITED); if (rc < 0) return TRUE; /* End of file or error */ smtp_ungetc(rc); -rc = smtp_inend - smtp_inptr; -if (rc > 150) rc = 150; -smtp_inptr[rc] = 0; return FALSE; } static BOOL check_sync(void) { -if (!smtp_enforce_sync || sender_host_address == NULL || sender_host_notsocket) +if (!smtp_enforce_sync || !sender_host_address || sender_host_notsocket) return TRUE; return wouldblock_reading(); @@ -439,9 +436,10 @@ if (!smtp_out) return FALSE; fflush(smtp_out); if (smtp_receive_timeout > 0) alarm(smtp_receive_timeout); -/* Limit amount read, so non-message data is not fed to DKIM */ +/* Limit amount read, so non-message data is not fed to DKIM. +Take care to not touch the safety NUL at the end of the buffer. */ -rc = read(fileno(smtp_in), smtp_inbuffer, MIN(IN_BUFFER_SIZE, lim)); +rc = read(fileno(smtp_in), smtp_inbuffer, MIN(IN_BUFFER_SIZE-1, lim)); save_errno = errno; alarm(0); if (rc <= 0) @@ -1614,11 +1612,11 @@ if (proxy_session && proxy_session_failed) /* Enforce synchronization for unknown commands */ -if (smtp_inptr < smtp_inend && /* Outstanding input */ - check_sync && /* Local flag set */ - smtp_enforce_sync && /* Global flag set */ - sender_host_address != NULL && /* Not local input */ - !sender_host_notsocket) /* Really is a socket */ +if ( smtp_inptr < smtp_inend /* Outstanding input */ + && check_sync /* Local flag set */ + && smtp_enforce_sync /* Global flag set */ + && sender_host_address /* Not local input */ + && !sender_host_notsocket) /* Really is a socket */ return BADSYN_CMD; return OTHER_CMD; @@ -2419,10 +2417,12 @@ else (sender_host_address ? protocols : protocols_local) [pnormal]; /* Set up the buffer for inputting using direct read() calls, and arrange to -call the local functions instead of the standard C ones. */ +call the local functions instead of the standard C ones. Place a NUL at the +end of the buffer to safety-stop C-string reads from it. */ if (!(smtp_inbuffer = US malloc(IN_BUFFER_SIZE))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "malloc() failed for SMTP input buffer"); +smtp_inbuffer[IN_BUFFER_SIZE-1] = '\0'; receive_getc = smtp_getc; receive_getbuf = smtp_getbuf; @@ -5669,8 +5669,8 @@ while (done <= 0) case BADCHAR_CMD: done = synprot_error(L_smtp_syntax_error, 0, NULL, /* Just logs */ - US"NULL character(s) present (shown as '?')"); - smtp_printf("501 NULL characters are not allowed in SMTP commands\r\n", FALSE); + US"NUL character(s) present (shown as '?')"); + smtp_printf("501 NUL characters are not allowed in SMTP commands\r\n", FALSE); break; @@ -5679,7 +5679,7 @@ while (done <= 0) if (smtp_inend >= smtp_inbuffer + IN_BUFFER_SIZE) smtp_inend = smtp_inbuffer + IN_BUFFER_SIZE - 1; c = smtp_inend - smtp_inptr; - if (c > 150) c = 150; + if (c > 150) c = 150; /* limit logged amount */ smtp_inptr[c] = 0; incomplete_transaction_log(US"sync failure"); log_write(0, LOG_MAIN|LOG_REJECT, "SMTP protocol synchronization error " diff --git a/test/stderr/0435 b/test/stderr/0435 index 1ec601657..ad55c7e8f 100644 --- a/test/stderr/0435 +++ b/test/stderr/0435 @@ -15,8 +15,8 @@ SMTP>> 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 smtp_setup_msg entered SMTP<< HELO ? LOG: smtp_syntax_error MAIN - SMTP syntax error in "HELO ?" U=CALLER NULL character(s) present (shown as '?') -SMTP>> 501 NULL characters are not allowed in SMTP commands + SMTP syntax error in "HELO ?" U=CALLER NUL character(s) present (shown as '?') +SMTP>> 501 NUL characters are not allowed in SMTP commands SMTP<< quit SMTP>> 221 myhost.test.ex closing connection LOG: smtp_connection MAIN diff --git a/test/stdout/0435 b/test/stdout/0435 index 2b974e2cf..c86a50837 100644 --- a/test/stdout/0435 +++ b/test/stdout/0435 @@ -1,3 +1,3 @@ 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
-501 NULL characters are not allowed in SMTP commands
+501 NUL characters are not allowed in SMTP commands
221 myhost.test.ex closing connection
|