386860 - [SECURITY] Insufficient escaping of From address when using Sendmail

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Loïc Minier]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.4) Gecko/20070508 (Debian-1.8.1.4-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<[e-mail]>'

The mail data looks something like:
From [e-mail] Wed Jul  4 17:19:58 2007
Date: Wed, 4 Jul 2007 17:19:58 +0200
From: =?iso-8859-1?Q?Lo=EFc?= Minier <[e-mail]>
To: [e-mail]
Subject: Essai
Message-ID: <[e-mail]>
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 '<[e-mail]>'
    to 'Loïc Minier <[e-mail]>':error when closing pipe to /usr/lib/sendmail:


Reproducible: Always

Comment 1

[Max Kanat-Alexander]

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.

Comment 2

[Loïc Minier]

Also happens with Email::Send 2.185.

Comment 3

[Loïc Minier]

(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.

Comment 4

[Max Kanat-Alexander]

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.

Comment 5

[Max Kanat-Alexander]

Attachment: v1

This should do it.

Comment 6

[Loïc Minier]

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: <[e-mail]>
^MTo: =?UTF-8?Q?Lo=C3=AFc=20Minier=20?=<[e-mail]>
^MSubject: Re: Essai
^MIn-Reply-To: <[e-mail]>
...
in mailer.testfile (note the first line without \r); this doesn't look like a
security issue though.

Comment 7

[Loïc Minier]

(In reply to comment #5)
> Created an attachment (id=270917) [details]
> v1
> 
> This should do it.

Fixes the first bug for me; thanks.

Comment 8

[Max Kanat-Alexander]

(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 9

[Frédéric Buclin]

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.

Comment 10

[Max Kanat-Alexander]

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.

Comment 11

[Max Kanat-Alexander]

Attachment: v2

Okay, good point.

Comment 12

[Max Kanat-Alexander]

Attachment: Actual v2

Oops, attached the same patch twice, there! :-)

Comment 13

[Frédéric Buclin]

Comment on attachment 270923 [details] [diff] [review]
Actual v2

Looks good. I assume you tested it. r=LpSolit

Comment 14

[Loïc Minier]

It's been 4 months with a remote command execution hole; do you plan to roll a tarball?

Comment 15

[Max Kanat-Alexander]

(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

Comment 16

[Frédéric Buclin]

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: 1.7.2.5; previous revision: 1.7.2.4
done

Comment 17

[Max Kanat-Alexander]

Security advisory from Bug 389843 sent, unlocking bug.