Add nullmailer - Githubissues

schongallam commented 2 weeks ago

I started a build of this container, but it needs more work. Background:

Nothing has been really updated in ~6 years (v2.2 is latest tagged version)
It builds fine in smtp-garden-soil
nullmailer-send is a persistent executable that just scans the queue and sends any new mail to a smarthost.
nullmailer-smtpd is neither an atomic SMTP server nor a daemon. It is basically just an SMTP-syntaxed wrapper for nullmailer-queue. It responds to HELO, MAIL, RCPT, DATA etc. Once you send the string 'DATA', it forks nullmailer-queue to handle everything until you send the terminating '.', then it takes over again to handle SMTP syntax.

Issues so far:

nullmailer-smtpd needs to be piped to a socket for it to communicate outside its container. My naive attempts with netcat were unsuccessful. Update: new dockerfile line CMD nullmailer-send & nc -lk -p 25 -c nullmailer-smtpd works.
Message body format for the DATA part needs to be confirmed. i.e., When feeding nullmailer-smtpd commands from stdin, in takes HELO, MAIL, RCPT, etc just fine. But after sending DATA, you have to enter the subsequent message body in a manner that nullmailer-queue expects, and I haven't worked that out yet. (It's probably just standard message body, though). When you don't do it right, nullmailer quits (both -queue and -smtpd) with an undocumented, non-zero status. In other words, a bare-bones DATA payload won't work at all.

Edit to add: Additional issue: If you send nothing but a '.' after DATA, then nullmailer-smtpd segfaults.

schongallam commented 2 weeks ago

Update: If you send DATA to nullmailer-smtpd that includes actual email header lines, and "MAIL FROM" and "RCPT TO" have FQDN addresses, then nullmailer-smtpd will relay the mail to echo. But if it doesn't like the headers, nullmailer-queue will fail and either quits silently (as best) or messily (segmentation fault).

For example, sending this works: ...MAIL FROM:<admin@root.edu>... But this fails: ...MAIL FROM:<admin@root>...

So, it works for us, but some major flaws are reflected in nullmailer.

I'll push another test message (with headers in the payload), and the updated docker files. I'll close this issue in a few days if nobody follows up on this.

kenballus commented 2 weeks ago

This all sounds like great motivation to keep Nullmailer around. Low-quality, widely-used software is exactly what we're looking for.

Additional issue: If you send nothing but a '.' after DATA, then nullmailer-smtpd segfaults.

This is a bug that we should report :)

kenballus commented 2 weeks ago

Also, plenty of projects make infrequent releases. See https://github.com/h2o/h2o, which hasn't had a release since 2019, but sees lots of activity and use.

schongallam commented 2 weeks ago

Signal handling added for graceful docker shutdown (dde8400)

schongallam commented 2 weeks ago

I seem to have forgotten to push the launcher script that went along with the updated Dockerfile. The script is included in the latest pull request. I'll close this issue once that is added and my bug report is filed.

schongallam commented 1 week ago

After some brute-forcing, I confirmed that the segfault can be reliably reproduced if the MAIL FROM or the RCPT TO address lacks a '.' character. For instance, 'aspan>@</spanb.c' works, 'a@b.' works, and even 'a@.' works. But without the '.', it will segfault when you try to close the DATA phase with '\r\n.\r\n'.

I recompiled nullmailer with clang++. Interestingly, C++17, C++14, and C++11 throw errors, which are not elicited when compiling with g++:

/usr/bin/clang++ -DHAVE_CONFIG_H -I. -I..  -I../lib   -fsanitize=address -fno-omit-frame-pointer -fno-rtti -fno-exceptions -W -Wall -MT inject.o -MD -MP -MF .deps/inject.Tpo -c -o inject.o inject.cc
inject.cc:211:3: error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long') to 'unsigned int' in initializer list [-Wc++11-narrowing]
  211 |   X(Sender,            T,F,F,F,F), // 0
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

But clang will ignore these problems and compile under C++03. Then I can enable ASan and it yields:

nullmailer-1  | [nullmailer] starting...
nullmailer-1  | [nullmailer] online
nullmailer-1  | Rescanning queue.
[I send an evil, segfault-causing payload here]
nullmailer-1  | AddressSanitizer:DEADLYSIGNAL
nullmailer-1  | =================================================================
nullmailer-1  | ==61==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7fe2bb913936 bp 0x7fff1c051a50 sp 0x7fff1c051208 T0)
nullmailer-1  | ==61==The signal is caused by a READ memory access.
nullmailer-1  | ==61==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
nullmailer-1  |     #0 0x7fe2bb913936  (/lib/x86_64-linux-gnu/libc.so.6+0xad936) (BuildId: d66c2f639cbba67fc6461d75acbd0087169bc2f1)
nullmailer-1  |     #1 0x5647c1a12b7e in strlen (/usr/local/bin/nullmailer-smtpd+0x45b7e) (BuildId: 2eb8321e7e68c1990c6ff4fce0a9b2e1c1f66656)
nullmailer-1  |     #2 0x5647c1adb498 in fdobuf::operator<<(char const*) (/usr/local/bin/nullmailer-smtpd+0x10e498) (BuildId: 2eb8321e7e68c1990c6ff4fce0a9b2e1c1f66656)
nullmailer-1  |     #3 0x5647c1ae16fa in fork_exec::wait() (/usr/local/bin/nullmailer-smtpd+0x1146fa) (BuildId: 2eb8321e7e68c1990c6ff4fce0a9b2e1c1f66656)
nullmailer-1  |     #4 0x5647c1ada89b in DATA(mystring&) smtpd.cc
nullmailer-1  |     #5 0x5647c1ada34d in dispatch() smtpd.cc
nullmailer-1  |     #6 0x5647c1ad9d07 in main (/usr/local/bin/nullmailer-smtpd+0x10cd07) (BuildId: 2eb8321e7e68c1990c6ff4fce0a9b2e1c1f66656)
nullmailer-1  |     #7 0x7fe2bb88fdb9  (/lib/x86_64-linux-gnu/libc.so.6+0x29db9) (BuildId: d66c2f639cbba67fc6461d75acbd0087169bc2f1)
nullmailer-1  |     #8 0x7fe2bb88fe74 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e74) (BuildId: d66c2f639cbba67fc6461d75acbd0087169bc2f1)
nullmailer-1  |     #9 0x5647c19fb750 in _start (/usr/local/bin/nullmailer-smtpd+0x2e750) (BuildId: 2eb8321e7e68c1990c6ff4fce0a9b2e1c1f66656)
nullmailer-1  | 
nullmailer-1  | AddressSanitizer can not provide additional info.
nullmailer-1  | SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0xad936) (BuildId: d66c2f639cbba67fc6461d75acbd0087169bc2f1) 
nullmailer-1  | ==61==ABORTING

To muddy the waters a little bit, nullmailer compiled by c++03 is unstable (i.e. it crashes after successfully sending a non-segfault-triggering payload email to echo, with pretty much the same fault trace). That being said... I'm going to assume for now that the known segfaulting behavior, with evil payloads, under gcc, is the same as under clang c++03, and that clang merely uncovers additional problems (as opposed to creating an entirely new bug out of the blue).

In other words, MAIL FROM:<a@b>\n...\DATA\n\r\n.\r\n -> segfault immediately MAIL FROM:<a@b.c>\n...\DATA\n\r\n.\r\n -> segfault after sending email to echo (clang/c++03 only)

So there's something going on with strlen() and casting an unsigned long to an unsigned int, for sure. But, that would be weird if it directly had anything to do with whether or not a '.' was present in the string. Maybe strlen is called in a different order, depending on parsing results of the MAIL FROM string? Unfortunately, I don't have time to review the code deeply at this moment. Any other theories?

I can push a testing branch with a nullmailer:asan image if you want something reproducible. I suppose there's enough to file a basic bug report, though, even without confirming the exact source error or suggesting a fix. Although correctly storing strlen's return size would be a good start.

schongallam commented 5 days ago

Update: nullmailer compiles under clang++17 by simply fixing line 153 of inject.cc: unsigned length becomes size_t length Program crashing and ASan output are unchanged, however. Another update: I've found the root cause and a fix, I'll post it here after I finish drafting the bug report for the author repo

schongallam commented 3 days ago

As it turns out, there was already a recently-opened nullmailer issue for this segfault, and the debian bug report logs did some good investigative work on this too. The bug was discovered by a different method, but they all basically boil down to: src/smtpd.cc declared a variable as a char[], when a char* should have been used, which resulted in an undefined pointer value being passed to strlen, resulting in an out-of-bounds memory read.

My testing expanded the existing bug report to a more generalized case, so I replied in a comment to the existing issue with further information. I also opened an issue about a very minor type inconsistency.

A patched version of nullmailer no longer segfaults (see forthcoming commit). A new bug is now unmasked, however. Consider the following two payload deliveries:

**[ First Payload ]**
root@f4e4468f6b1c:/app# nullmailer-smtpd
220 nullmailer-smtpd ready
HELO minimal-DATA-example
250 2.3.0 OK
MAIL FROM:<a@b>
250 2.1.0 Sender accepted
RCPT TO:<x@y>
250 2.1.5 Recipient accepted
DATA
354 End your message with a period on a line by itself
.
nullmailer-smtpd: nullmailer-queue failed: 1
451 4.3.0 Error returned from nullmailer-queue
QUIT
221 2.0.0 Good bye
root@f4e4468f6b1c:/app# echo $?
0
**[ Second Payload ]**
root@f4e4468f6b1c:/app# nullmailer-smtpd
220 nullmailer-smtpd ready
HELO try-to-send-any-DATA
250 2.3.0 OK
MAIL FROM:<a@b>
250 2.1.0 Sender accepted
RCPT TO:<x@y>
250 2.1.5 Recipient accepted
DATA
354 End your message with a period on a line by itself
any text here, including: valid header lines
root@f4e4468f6b1c:/app# echo $?
141

(Not shown: if MAIL/RCPT are fully qualified RFC 5321 addresses, nullmailer continues to relay the message as normal, like before.)

The first payload reflects expected behavior: malformed MAIL/RCPT values and empty DATA content. Makes total sense that it's rejected.

The second payload demonstrates that, with a malformed MAIL/RCPT string, upon trying to send any DATA other than an immediate terminating \r\n.\r\n, the program quits with an undocumented '141'.

I'll start looking into this next. Update: looks maybe to be an unhandled SIGPIPE signal. When valid FROM/RCPT values are given, the stream stays open and accepts the contents of DATA to be written. When invalid FROM/RCPT values have previously been sent to the stream, the stream silently closes, and the next write attempt by qwrite to the assigned qfd elicits the signal, exiting the program unexpectedly. Solution would be to confirm the stream is open before each write call (high overhead cost, vulnerable to race condition) or better yet to just catch the signal and handle it in the code. I'll test a POC and report it to the nullmailer author.

schongallam commented 3 days ago

Latent bug root cause confirmed, and reported as a new issue (#97) on nullmailer repo: SIGPIPE signal handling failure, resulting unexpected program exit.

kenballus / smtp-garden

Add nullmailer #13