reproducible crash when calling nullmailer-smtpd from swaks

[bremner]

In 1 Axel observes that

   swaks -t [email protected] --pipe '/usr/lib/sendmail -bs'

reliably segfaults nullmailer.

I duplicated this also with

   swaks -t [email protected] --pipe 'nullmailer-smtpd'

I ran nullmailer-smtpd under gdb and valgrind and it succeeded in both cases. I tried rebuilding with ASAN, and the test "Testing protocol success with smtp (stdin)" finds one memory leak.

    220 OK
    smtp: Succeeded: 220 OK

    =================================================================
    ==3035435==ERROR: LeakSanitizer: detected memory leaks

    Direct leak of 13 byte(s) in 1 object(s) allocated from:
        #0 0x7f64762edd10 in strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:578
        #1 0x563a16835173 in parse_option
/home/bremner/software/debian/nullmailer/protocols/protocol.cc:116
        #2 0x563a16835173 in parse_options
/home/bremner/software/debian/nullmailer/protocols/protocol.cc:130
        #3 0x563a16835173 in cli_main(int, char**)
/home/bremner/software/debian/nullmailer/protocols/protocol.cc:138

    SUMMARY: AddressSanitizer: 13 byte(s) leaked in 1 allocation(s).

Looking at the code there is indeed a leak, but it's hard to see how it
would lead to a crash.

[bernhardu]

There are new findings in the Debian bug report
which lead to the crash happens when an error message should be printed.

Program received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
76      in ../sysdeps/x86_64/multiarch/strlen-avx2.S
1: x/i $pc
=> 0x7f0527973dd9 <__strlen_avx2+25>:   vpcmpeqb (%rdi),%ymm0,%ymm1
(rr) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1  0x00005639bd91306e in fdobuf::operator<< (str=0x6c69616d6c6c756e <error: Cannot access memory at address 0x6c69616d6c6c756e>, this=<optimized out>) at ./fdbuf/fdobuf.h:59
#2  fork_exec::wait (this=this@entry=0x7ffc4099cd00) at ./lib/forkexec.cc:125
#3  0x00005639bd91201f in DATA (param=...) at ./src/smtpd.cc:159
#4  DATA (param=...) at ./src/smtpd.cc:127
#5  0x00005639bd91144f in dispatch () at ./src/smtpd.cc:252
#6  main () at ./src/smtpd.cc:263

A git bisect showed 5850a49 as the first commit causing this crash.

Following diff would avoid the crash:

--- src/smtpd.cc.orig   2023-04-22 19:06:36.000000000 +0200
+++ src/smtpd.cc        2024-06-21 10:45:25.982395298 +0200
@@ -53,5 +53,5 @@ static mystring sender;
 static mystring recipients;
 
-extern const char cli_program[] = "nullmailer-smtpd";
+const char* cli_program = "nullmailer-smtpd";
 
 static int readline()

[schongallam]

I also recently stumbled across this bug, through a different mechanism. As bernhardu alludes to, it is a result of any error message printed to ferr by smtpd where cli_program is involved, not just swaks or sendmail related things. The type confusion between char* and char[] is indeed the root cause.

Any SMTP payload that causes fork_exec::wait_status to return a non-zero value to fork_exec::wait should suffice to reproduce this crash.

General steps to reproduce:

• Any minimalist configuration for nullmailer should suffice.
• If you want use clang++ and ASan, you either need to use -Wno-c++11-narrowing, or the C++03 standards, or patch inject.cc to fix type mismatches (which could be its own separate issue item).
• Launch nullmailer-send in the background.
• Give nullmailer-smtpd any malformed payload that is sufficiently constructed to invoke nullmailer-queue, such as either the MAIL FROM or the RCPT TO target string lacks a '.' character (indicating the presence of a TLD).
• Minimalist example:

HELO x\n
MAIL FROM:<y>\n
RCPT TO:<z>\n
DATA\n
\r\n.\r\n

(Specifically, I confirmed this behavior building with both nullmailer with gcc and clang++17 in a debian:trixie-slim container. /usr/local/etc/nullmailer/remotes is a one-liner consisting of ${RELAYHOST} smtp)

Expected behavior: Malformed MAIL/RCPT addresses that are not FQDN should be rejected, in accordance with RFC 5321.

• fork_exec::wait should report an error message via ferr.
• nullmailer-smtpd should report an SMTP 451 error message (or possibly a 553 error)
• For example:

nullmailer-smtpd: nullmailer-queue failed: 1
451 4.3.0 Error returned from nullmailer-queue

Resulting behavior: Segmentation fault.

• My backtraces are similar to berhardu's, implicating the overloaded << operator which is naive to char[] types.
• An undefined pointer value is passed to strlen, which attempts an out-of-bounds memory read (lib/forkexec.cc line 129)

Impact:

• Clients of nullmailer-smtpd are able to cause failure of nullmailer-smtpd.
• Undefined pointer dereferences may also theoretically, if paired with other vulnerabilities, reveal sensitive memory contents (though I am not aware of any POC for this example).

Fix(es):

• The simplest fix is bernhardu's proposed diff, above. smtpd.cc is the only one of 11 source files defining cli_program as an array instead of a pointer.
• An alternate fix is to additionally overload << to correctly handle the char[] type, if such a capacity is desired.

[bruceg]

Thank you for the extensive report, all. This is indeed a silly typo and should be fixed in 79a8a45

Note that the man page explicitly cautions not to use this in an untrusted environment. As such, while I fully agree this is a bug that needs fixing, I am not seeing how this can be used to cause a security issue.