User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:220.127.116.11) Gecko/20070508 (Debian-18.104.22.168-3) Galeon/2.0.2 (Debian package 2.0.2-4) Build Identifier: 3.0 email_reply.pl performs insufficient escaping of arguments passed to sh -c sendmail. This might be remotely exploitable; setting security flag. Long story: After some hours battling, I could get email_reply.pm to run, but when passed valid mail data, it would fail with: sh: -c: line 0: syntax error near unexpected token `newline' sh: -c: line 0: `/usr/lib/sendmail -t -oi -i -f<email@example.com>' The mail data looks something like: From firstname.lastname@example.org Wed Jul 4 17:19:58 2007 Date: Wed, 4 Jul 2007 17:19:58 +0200 From: =?iso-8859-1?Q?Lo=EFc?= Minier <email@example.com> To: firstname.lastname@example.org Subject: Essai Message-ID: <20070704151958.GD9800@bee.dooz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit Essai -- Loïc Minier I worked around this problem by adding single quotes to: push(@args, "-f'$from'") if $from; in Bugzilla/Mailer.pm, around line 80. This is with version 2.181 of Email::Send as found in Debian stable (etch) in the libemail-send-perl package. Note: email_reply.pl still fails for me afterwards with: sendmail: fatal: No recipient addresses found in message header There was an error sending mail from '<email@example.com>' to 'Loïc Minier <firstname.lastname@example.org>':error when closing pipe to /usr/lib/sendmail: Reproducible: Always
Do you mean Email::Reply? Or email_in.pl? Email::Reply isn't maintained by us, it's a CPAN module. email_in.pl is the name of the Bugzilla script.
Also happens with Email::Send 2.185.
(In reply to comment #1) > Do you mean Email::Reply? Or email_in.pl? > > Email::Reply isn't maintained by us, it's a CPAN module. > > email_in.pl is the name of the Bugzilla script. I meant email_in.pl, sorry for the typo; the actual bug seems to be in Bugzilla/Mailer.pm though.
Normally this isn't as much of a problem, because the only source of the mailfrom parameter is Bugzilla itself. However, in a shared hosting environment that would still be a problem. In email_in.pl, though, Email::Reply uses the "To" address as the "From" address for replies. So that's user-submitted data, and could be a problem. The solution here is to use Email::Address to parse the $from when setting -f for sendmail, and just include the email address. We probably should also do some escaping of common shell characters, but that's a bit trickier, since I don't know of any function or module that will do that for us.
Created attachment 270917 [details] [diff] [review] v1 This should do it.
Ack on the analysis; I wasn't versed enough to use one the dozen of Email:: and Mail:: functions to properly extract the email address, but I think email address can be quite complex if you honor all RFC 822/2822 trickiness, so without solid shell escaping, it is very risky. As an alternate resolution, I suggest using the configured mailfrom instead of reading the To:. Should I file a separate issue for the remaining problem I get afterwards? It seems due to a malformed email message text, presumably CRLF/LF mismatch in line-endings since I see: From: <email@example.com> ^MTo: =?UTF-8?Q?Lo=C3=AFc=20Minier=20?=<firstname.lastname@example.org> ^MSubject: Re: Essai ^MIn-Reply-To: <20070704151958.GD9800@bee.dooz.org> ... in mailer.testfile (note the first line without \r); this doesn't look like a security issue though.
(In reply to comment #5) > Created an attachment (id=270917) [details] > v1 > > This should do it. Fixes the first bug for me; thanks.
(In reply to comment #6) > As an alternate resolution, I suggest using the configured mailfrom instead of > reading the To:. I want to preserve the "To" that people wrote, in the actual email, but use just the email address on the command line. > Should I file a separate issue for the remaining problem I get afterwards? > [snip] Those look like a possible problem in Email::Reply. Perhaps contact the author of that module.
Comment on attachment 270917 [details] [diff] [review] v1 >+ my ($email_obj) = Email::Address->parse($from); >+ my $from_email = $email_obj->address; If I pass an invalid string, $email_obj is undefined and you cannot call address(). You should probably write: |$email_obj || return| or something like that.
Okay, further testing shows that this is not exploitable on 2.22 or 2.20, because we use Mail::Send there and it uses the multiple-argument form of exec(). It *is* exploitable on 3.0, since 2.23.4. (Actually since bug 353711.) We may want to mention this to rjbs (the maintainer of Email::Send) to put in the POD for Email::Send::Sendmail.
Created attachment 270922 [details] [diff] [review] v2 Okay, good point.
Created attachment 270923 [details] [diff] [review] Actual v2 Oops, attached the same patch twice, there! :-)
Comment on attachment 270923 [details] [diff] [review] Actual v2 Looks good. I assume you tested it. r=LpSolit
It's been 4 months with a remote command execution hole; do you plan to roll a tarball?
(In reply to comment #14) > It's been 4 months with a remote command execution hole; do you plan to roll a > tarball? Hi Loic. Yes, we do. You can track the process of Bugzilla releases on the bugs blocking this one: https://bugzilla.mozilla.org/show_bug.cgi?id=bz-release
tip: Checking in Bugzilla/Mailer.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Mailer.pm,v <-- Mailer.pm new revision: 1.12; previous revision: 1.11 done 3.0: Checking in Bugzilla/Mailer.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Mailer.pm,v <-- Mailer.pm new revision: 22.214.171.124; previous revision: 126.96.36.199 done
Security advisory from Bug 389843 sent, unlocking bug.