Closed Bug 386860 Opened 17 years ago Closed 17 years ago

[SECURITY] Insufficient escaping of From address when using Sendmail

Categories

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

2.23.4

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: lool, Assigned: mkanat)

References

Details

(Whiteboard: [ready for 3.x])

Attachments

(1 file, 2 obsolete files)

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<submit@bugzilla.example.com>'

The mail data looks something like:
From lool@dooz.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 <lool@dooz.org>
To: submit@bugzilla.example.com
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 '<submit@bugzilla.bpl-group.org>'
    to 'Loïc Minier <lool@dooz.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.
Summary: Insufficient escaping in email_reply.pl → Insufficient escaping in email_in.pl / Bugzilla/Mailer.pm
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.
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking3.1.1+
Flags: blocking3.0.1+
Priority: -- → P1
Summary: Insufficient escaping in email_in.pl / Bugzilla/Mailer.pm → Insufficient escaping of From address when using email_in.pl / sending emails
Target Milestone: --- → Bugzilla 3.0
Attached patch v1 (obsolete) — Splinter Review
This should do it.
Assignee: general → mkanat
Status: NEW → ASSIGNED
Attachment #270917 - Flags: review?(LpSolit)
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: <submit@bugzilla.example.com>
^MTo: =?UTF-8?Q?Lo=C3=AFc=20Minier=20?=<lool@dooz.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.
Version: unspecified → 2.23.4
Attached patch v2 (obsolete) — Splinter Review
Okay, good point.
Attachment #270917 - Attachment is obsolete: true
Attachment #270922 - Flags: review?(LpSolit)
Attachment #270917 - Flags: review?(LpSolit)
Summary: Insufficient escaping of From address when using email_in.pl / sending emails → Insufficient escaping of From address when using Sendmail
Attached patch Actual v2Splinter Review
Oops, attached the same patch twice, there! :-)
Attachment #270922 - Attachment is obsolete: true
Attachment #270923 - Flags: review?(LpSolit)
Attachment #270922 - Flags: review?(LpSolit)
Comment on attachment 270923 [details] [diff] [review]
Actual v2

Looks good. I assume you tested it. r=LpSolit
Attachment #270923 - Flags: review?(LpSolit) → review+
Flags: approval?
Flags: approval3.0?
Whiteboard: [ready for 3.x]
Blocks: 389843
Summary: Insufficient escaping of From address when using Sendmail → [SECURITY] Insufficient escaping of From address when using Sendmail
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
Flags: approval?
Flags: approval3.0?
Flags: approval3.0+
Flags: approval+
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Security advisory from Bug 389843 sent, unlocking bug.
Group: webtools-security
You need to log in before you can comment on or make changes to this bug.