(possible) DOS for malformed mails containing NULL-bytes in body

[Rolf Weber]

Hi!

I’ve run across an issue where piler-smtp hangs for certain malformed mails. The mail in question had a malformed attachment:

Content-Type: application/octet-stream; name="xxx.png"
Content-Transfer-Encoding: 7bit
Content-ID: <logo>
Content-Disposition: attachment; filename="xxx.png"

which was in fact just binary data containing (at least) a NULL-byte. Postfix, amavis/spammassassin and Exchange have no problem proccessing these mails, spamassassin even has an NULL_IN_BODY Rule.

I’ve tracked the issue down to read_one_line() in misc.c called from handle_data() in session.c as follows:
1. read_one_line() will copy the start of the line from char *s to char *buf until it reaches the NULL the first time it is called (for loop is broken by ‘*s == NULL’ once reached) and returns the number of chars copied, rc = ERR.
2. do-loop in session.c calls read_one_line() once more, this will immediately return 0, making handle_data() return.

At this point the buffer in handle_data() still contains some data that has not been copied (and never will be, even when called again).

‌

Possible fix: evaluate rc in the do{}, it should be ERR in this case, and make handle_data() return an error, and stop processing of the broken mail.
I’m not familiar enough with the code to guess if this would work.

piler 1.3.6, build 998, Janos SUTO [e-mail] - but irrelevant, I’ve checked against master for the links above.

‌

[Janos SUTO]

Can you attach such email to the issue to verify and fix it with?

[Rolf Weber]

stripped of most headers and mail content. NULL in attachment should still trigger the behavior

[Janos SUTO]

Thank you. Anyway, it’s odd that this mail client failed to encode the binary attachment. I’ll come up with a fix soon, and let you know.

[Rolf Weber]

These are automated mails sent by some web portal via AWS. Looks like something is broken there (I have notified them).

This should however not lead to piler-smtp being blocked by the mail content. piler-smtp is processing totally untrusted input and should not make any assumptions about its contents and whether the input is valid.
read_one_line() is basically an extended strncpy() with additional stop chars, so it assumes there will never be NULL in the input.

[Rolf Weber]

insert missing newline

[Janos SUTO]

https://bitbucket.org/jsuto/piler/commits/e1ea14374a57ae4a517a789b298620ecc9fa7b7f

This commit should fix the issue. When receiving such a message, piler-smtp now properly receives it, then moves it to /var/piler/error along with a syslog message like

ERROR: 2UERGKNS5GWF16CA contains an invalid NUL-byte, moving it to /var/piler/error

Btw. I managed to import this attached test email in the test environment, however, the png attachment became garbage. Not sure if it’s worth trying to fix to retrieve such message from the archive. That’s the reasoning for moving to the error dir for later inspection.

[Rolf Weber]

I just skimmed the diffs, should work.
Testing now.

‌

I’ve had some trouble rebuilding on debian/stretch (needs to be upgraded soonish) and had to disable TLS1.3:

gcc -std=c99 -O2 -fPIC -Wall -Wextra  -Wuninitialized -Wno-format-truncation -g  -I. -I..  -I/usr/include/mysql  -D_GNU_SOURCE -DHAVE_TRE -DNEED_MYSQL -c cfg.c -o cfg.o
cfg.c: In function ‘get_tls_protocol_number’:
cfg.c:156:20: error: ‘TLS1_3_VERSION’ undeclared (first use in this function)
       { "TLSv1.3", TLS1_3_VERSION },
                    ^~~~~~~~~~~~~~

# openssl version
OpenSSL 1.1.0l  10 Sep 2019

# apt-cache policy libssl-dev
libssl-dev:
  Installed: 1.1.0l-1~deb9u3
  Candidate: 1.1.0l-1~deb9u3

# grep OPENSSL_VERSION_NUMBER /usr/include/openssl/opensslv.h
# define OPENSSL_VERSION_NUMBER  0x101000cfL

‌

piler/src/cfg.c:
155 #if OPENSSL_VERSION_NUMBER >= 0x10100000L
156 { "TLSv1.3", TLS1_3_VERSION },
157 #endif

I didn’t check what the minimum OPEN_SSL_VERSION_NUMBER is supposed to be for TLS1_3_VERSION to be available.

[Janos SUTO]

Try the following instead:

#ifdef TLS1_3_VERSION
  { "TLSv1.3", TLS1_3_VERSION },
#endif

‌

[Rolf Weber]

This was meant as a side note and is somewhat off-topic.

TLS1.3 support was introduced in OpenSSL 1.1.1, so the check should probably be

#if OPENSSL_VERSION_NUMBER >= 0x10101000L

‌

[Janos SUTO]

I think it’s better to test for the given feature, not for version number.

[Janos SUTO]

Anyway, I believe that the original issue has been fixed.