Gitweb:
https://git.exim.org/exim.git/commitdiff/36771878fa93a04ecf5bdd71ad3c3c380a16aa03
Commit: 36771878fa93a04ecf5bdd71ad3c3c380a16aa03
Parent: 518f0a0dd6df6f0d0ea51bfa126982d134e7a7ff
Author: Phil Pennock <phil+git@???>
AuthorDate: Thu Oct 29 23:21:36 2020 -0400
Committer: Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Thu May 27 21:30:31 2021 +0200
SECURITY: rework BDAT receive function handling
(cherry picked from commit dd1b9b753bb7c42df2b8f48d726b82928b67940b)
(cherry picked from commit 96fb195ebc2eb6790e6ad6dde46d478aee62198d)
---
doc/doc-txt/ChangeLog | 6 +++++
src/src/globals.c | 1 +
src/src/smtp_in.c | 67 ++++++++++++++++++++++++++++++++++++---------------
3 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 9837d6c..0e008c9 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -288,6 +288,12 @@ PP/09 Fix security issue with too many recipients on a message (to remove a
PP/10 Fix security issue in SMTP verb option parsing
Fixes CVE-2020-EXOPT reported by Qualys.
+PP/11 Fix security issue in BDAT state confusion.
+ Ensure we reset known-good where we know we need to not be reading BDAT
+ data, as a general case fix, and move the places where we switch to BDAT
+ mode until after various protocol state checks.
+ Fixes CVE-2020-BDATA reported by Qualys.
+
Exim version 4.94
-----------------
diff --git a/src/src/globals.c b/src/src/globals.c
index bd874a7..e1837b6 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -227,6 +227,7 @@ struct global_flags f =
.authentication_local = FALSE,
.background_daemon = TRUE,
+ .bdat_readers_wanted = FALSE,
.chunking_offered = FALSE,
.config_changed = FALSE,
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 4f16fd4..eb032bb 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -624,9 +624,7 @@ for(;;)
if (chunking_data_left > 0)
return lwr_receive_getc(chunking_data_left--);
- receive_getc = lwr_receive_getc;
- receive_getbuf = lwr_receive_getbuf;
- receive_ungetc = lwr_receive_ungetc;
+ bdat_pop_receive_functions();
#ifndef DISABLE_DKIM
dkim_save = dkim_collect_input;
dkim_collect_input = 0;
@@ -730,9 +728,7 @@ next_cmd:
goto repeat_until_rset;
}
- receive_getc = bdat_getc;
- receive_getbuf = bdat_getbuf; /* r~getbuf is never actually used */
- receive_ungetc = bdat_ungetc;
+ bdat_push_receive_functions();
#ifndef DISABLE_DKIM
dkim_collect_input = dkim_save;
#endif
@@ -765,9 +761,7 @@ while (chunking_data_left)
if (!bdat_getbuf(&n)) break;
}
-receive_getc = lwr_receive_getc;
-receive_getbuf = lwr_receive_getbuf;
-receive_ungetc = lwr_receive_ungetc;
+bdat_pop_receive_functions();
if (chunking_state != CHUNKING_LAST)
{
@@ -777,7 +771,35 @@ if (chunking_state != CHUNKING_LAST)
}
+void
+bdat_push_receive_functions(void)
+{
+/* push the current receive_* function on the "stack", and
+replace them by bdat_getc(), which in turn will use the lwr_receive_*
+functions to do the dirty work. */
+if (lwr_receive_getc == NULL)
+ {
+ lwr_receive_getc = receive_getc;
+ lwr_receive_getbuf = receive_getbuf;
+ lwr_receive_ungetc = receive_ungetc;
+ }
+else
+ {
+ DEBUG(D_receive) debug_printf("chunking double-push receive functions\n");
+ }
+receive_getc = bdat_getc;
+receive_ungetc = bdat_ungetc;
+}
+
+void
+bdat_pop_receive_functions(void)
+{
+receive_getc = lwr_receive_getc;
+receive_getbuf = lwr_receive_getbuf;
+receive_ungetc = lwr_receive_ungetc;
+lwr_receive_getc = lwr_receive_getbuf = lwr_receive_ungetc = NULL;
+}
/*************************************************
* SMTP version of ungetc() *
@@ -2528,6 +2550,7 @@ receive_ungetc = smtp_ungetc;
receive_feof = smtp_feof;
receive_ferror = smtp_ferror;
receive_smtp_buffered = smtp_buffered;
+lwr_receive_getc = lwr_receive_getbuf = lwr_receive_ungetc = NULL;
smtp_inptr = smtp_inend = smtp_inbuffer;
smtp_had_eof = smtp_had_error = 0;
@@ -3954,6 +3977,14 @@ cmd_list[CMD_LIST_EHLO].is_mail_cmd = TRUE;
cmd_list[CMD_LIST_STARTTLS].is_mail_cmd = TRUE;
#endif
+if (lwr_receive_getc != NULL)
+ {
+ /* This should have already happened, but if we've gotten confused,
+ force a reset here. */
+ DEBUG(D_receive) debug_printf("WARNING: smtp_setup_msg had to restore receive functions to lowers\n");
+ bdat_pop_receive_functions();
+ }
+
/* Set the local signal handler for SIGTERM - it tries to end off tidily */
had_command_sigterm = 0;
@@ -5288,16 +5319,7 @@ while (done <= 0)
DEBUG(D_receive) debug_printf("chunking state %d, %d bytes\n",
(int)chunking_state, chunking_data_left);
- /* push the current receive_* function on the "stack", and
- replace them by bdat_getc(), which in turn will use the lwr_receive_*
- functions to do the dirty work. */
- lwr_receive_getc = receive_getc;
- lwr_receive_getbuf = receive_getbuf;
- lwr_receive_ungetc = receive_ungetc;
-
- receive_getc = bdat_getc;
- receive_ungetc = bdat_ungetc;
-
+ f.bdat_readers_wanted = TRUE;
f.dot_ends = FALSE;
goto DATA_BDAT;
@@ -5306,6 +5328,7 @@ while (done <= 0)
case DATA_CMD:
HAD(SCH_DATA);
f.dot_ends = TRUE;
+ f.bdat_readers_wanted = FALSE
DATA_BDAT: /* Common code for DATA and BDAT */
#ifndef DISABLE_PIPE_CONNECT
@@ -5334,7 +5357,10 @@ while (done <= 0)
: US"valid RCPT command must precede BDAT");
if (chunking_state > CHUNKING_OFFERED)
+ {
+ bdat_push_receive_functions();
bdat_flush_data();
+ }
break;
}
@@ -5373,6 +5399,9 @@ while (done <= 0)
}
}
+ if (f.bdat_readers_wanted)
+ bdat_push_receive_functions();
+
if (user_msg)
smtp_user_msg(US"354", user_msg);
else
