dovecot - Nov 2013 - sieve + notify + $subject$ + empty subject = segv

Hello,

we can reproduce a bug where lmtp crashes:

LMTP telnet session:
Trying 127.0.0.1...
Connected to localhost.localdomain (127.0.0.1).
Escape character is '^]'.
220 lisa.m-otion.at Dovecot ready.
lhlo xyz
250-xyz.m-otion.at
250-8BITMIME
250-ENHANCEDSTATUSCODES
250 PIPELINING
mail from:<xyz at m-otion.at>
250 2.1.0 OK
rcpt to:<my_mailbox>
250 2.1.5 OK
data
354 OK
From:<xyz at m-otion.at>

test
.
Connection closed by foreign host.

/var/log/messages tells me:
Nov 11 00:07:00 mail kernel: lmtp[15868]: segfault at 0000000000000000 rip
00002b1960d01d70 rsp 00007fff3fa95908 error 4

pigeonhole 0.4.2, dovecot 2.2.6

and this is what I found out:
If you use a sieve script containing the following line 
  notify :method "mailto" :options "xyz" :message
"$from$*#+$subject$";

and send a mail without a Subject: header line lmtp crashes. The bug is related
to the sieve module in /src/lib-sieve/plugins/notify/ext-notify-common.c:266.
The line contains only a check if mail_get_headers_utf8 returns an error. So the
workaround for this line would be to add a check if header[0] is not NULL.

                        if ( (mail_get_headers_utf8(msgdata->mail,
"subject", &header) >= 0) && header[0] )
                                 str_append(out_msg, header[0]);

IMHO this patch should be applied to the "$from$" replacement as well.
The comments in the dovecot's header file says for mail_get_headers:
   Returns -1 if error, 0 otherwise (with or without headers found).

hope it helps,

Regards
Christian Ferbar

On 11/11/2013 12:54 AM, Christian Ferbar wrote:
> Hello,
>
> we can reproduce a bug where lmtp crashes:
> If you use a sieve script containing the following line 
>   notify :method "mailto" :options "xyz" :message
"$from$*#+$subject$";
>
> and send a mail without a Subject: header line lmtp crashes. The bug is
related to the sieve module in
/src/lib-sieve/plugins/notify/ext-notify-common.c:266. The line contains only a
check if mail_get_headers_utf8 returns an error. So the workaround for this line
would be to add a check if header[0] is not NULL.
This is an old one. Good catch. This should fix it:

http://hg.rename-it.nl/dovecot-2.2-pigeonhole/rev/44f659981723

I solved it slightly differently. Also added tests.

Regards,

Stephan.