SECURITY: rework BDAT receive function handling

(cherry picked from commit dd1b9b753bb7c42df2b8f48d726b82928b67940b)
(cherry picked from commit 96fb195ebc2eb6790e6ad6dde46d478aee62198d)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 9837d6c0f61ac0e22322909d7d4d49d1a5520302..0e008c985742a55c362e67ccdc433b845a4ae3b5 100644 (file)
--- 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 bd874a789c8ba26d153a2406ad164d9c8f20beaa..e1837b67d5fcd8a92400e1d47356167083ec4561 100644 (file)
--- 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 4f16fd4b82f5de8e4fd1cc6fd072208882662e54..eb032bb52ee2ee7d8ead36faa4a0cd53fdc024f1 100644 (file)
--- 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