Bug 600464 (CVE-2010-3172)

[SECURITY] Content/Header injection due to non-random multipart/x-mixed-replace boundary

VERIFIED FIXED in Bugzilla 3.2

Status

()

defect
P1
critical
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: masa141421356, Assigned: glob)

Tracking

2.10
Bugzilla 3.2
Dependency tree / graph
Bug Flags:
approval +
approval4.0 +
blocking4.0 +
approval3.6 +
blocking3.6.3 +
approval3.4 +
blocking3.4.9 +
approval3.2 +
blocking3.2.9 +

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

9 years ago
Search result of b.m.o. does not escape "--------- =_aaaaaaaaaa0": it is used as boudary of multipart/x-mixed-replace.

Attackers can inject boundary of multipart/x-mixed-replace.
It may be able to be used for HTTP Header injection.

Example:
https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc=---------%20%3D_aaaaaaaaaa0&short_desc_type=allwordssubstr&resolution=---&resolution=DUPLICATE
Reporter

Comment 1

9 years ago
I think, there are two solutions.
1. Change boudary text to be random.
2. If search result contains boundary text, replace it to HTML character reference.
Flags: blocking4.0?
Flags: blocking3.6.3?
Flags: blocking3.4.9?
Flags: blocking3.2.9?
Summary: multipart/x-mixed-replace boudary Injection to search result of b.m.o. → multipart/x-mixed-replace boundary injection in search result
This is really a bug with upstream CGI module... The default boundary is hardcoded when it should be randomized.

sub multipart_init {
    my($self,@p) = self_or_default(@_);
    my($boundary,@other) = rearrange_header([BOUNDARY],@p);
    $boundary = $boundary || '------- =_aaaaaaaaaa0';
    $self->{'separator'} = "$CRLF--$boundary$CRLF";
    $self->{'final_separator'} = "$CRLF--$boundary--$CRLF";
    $type = SERVER_PUSH($boundary);
    return $self->header(
	-nph => 0,
	-type => $type,
	(map { split "=", $_, 2 } @other),
    ) . "WARNING: YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY." . $self->multipart_end;
}
Severity: normal → major
Version: unspecified → 3.6.2
Severity: major → critical
Priority: -- → P1
Summary: multipart/x-mixed-replace boundary injection in search result → [SECURITY] Content/Header injection due to non-random multipart/x-mixed-replace boundary
Target Milestone: --- → Bugzilla 3.2
Assignee

Comment 3

9 years ago
Posted patch randomise mime boundary.1 (obsolete) — Splinter Review
we already overwrite multipart_init, so the fix is trivial.
Assignee: query-and-buglist → glob
Status: NEW → ASSIGNED
Attachment #479324 - Flags: review?(mkanat)
Alias: CVE-2010-3172
Assignee

Comment 4

9 years ago
i have emailed the maintainers of CGI.pm to make them aware of this issue.
Well played, sir! Excellent bug-finding skills. 

The fix looks fine to me, although someone might claim there is a slightly more performant way to do it. But this code only runs once per request, so clarity probably wins. You might want to add a comment about why you are doing it.

Gerv

Comment 6

9 years ago
Comment on attachment 479324 [details] [diff] [review]
randomise mime boundary.1

>+        my @chrs = ('0'..'9', 'A'..'Z', 'a'..'z');
>+        foreach (0..16) {
>+            $boundary .= $chrs[rand(scalar @chrs)];
>+        }

This code already exists in generate_random_password(); no need to duplicate it. Please use it instead.
Attachment #479324 - Flags: review-
Assignee

Updated

9 years ago
Attachment #479324 - Flags: review?(mkanat)
Assignee

Comment 7

9 years ago
Attachment #479324 - Attachment is obsolete: true
Attachment #479344 - Flags: review?

Updated

9 years ago
Flags: blocking4.0?
Flags: blocking4.0+
Flags: blocking3.6.3?
Flags: blocking3.6.3+
Flags: blocking3.4.9?
Flags: blocking3.4.9+
Flags: blocking3.2.9?
Flags: blocking3.2.9+

Updated

9 years ago
Attachment #479344 - Flags: review? → review+

Updated

9 years ago
Flags: approval?
Flags: approval4.0?
Flags: approval3.6?
Flags: approval3.4?
Flags: approval3.2?
(In reply to comment #4)
> i have emailed the maintainers of CGI.pm to make them aware of this issue.

Did you let them know we already have a CVE number assigned for it?  I assume the number can be given to them at this point since it's actually they're bug and we're working around it.  Or does the fact that we were already overriding that subroutine make ours a separate issue?
(In reply to comment #6)
> Comment on attachment 479324 [details] [diff] [review]
> randomise mime boundary.1
> 
> >+        my @chrs = ('0'..'9', 'A'..'Z', 'a'..'z');
> >+        foreach (0..16) {
> >+            $boundary .= $chrs[rand(scalar @chrs)];
> >+        }
> 
> This code already exists in generate_random_password(); no need to duplicate
> it. Please use it instead.

Using our generate_random_password() routine is not upward-portable back to the upstream CGI module, however, since they don't have that routine.
(In reply to comment #9)
> Did you let them know we already have a CVE number assigned for it?  I assume
> the number can be given to them at this point since it's actually they're bug
> and we're working around it.  Or does the fact that we were already overriding
> that subroutine make ours a separate issue?

The CVE for this bug is solely for the Bugzilla-specific fix, as it's not actually patching the bug within CGI.pm but in our override function. However, if the CGI.pm maintainers need a CVE to be assigned for them, that can be easily arranged.
Reporter

Comment 13

9 years ago
CGI::Simple seems to have same problem.

http://cpansearch.perl.org/src/ANDYA/CGI-Simple-1.112/lib/CGI/Simple.pm
Reporter

Comment 14

9 years ago
FYI:
It might be better to write documentation that describes why randomization is needed.

I cannot find any documentation that describes "why boundary is needed to randomized on dynamic contents that uses multipart/x-mixed-replace".

Almost of documentations and example uses non-random boundary.
  for example: http://perldoc.perl.org/CGI.html#Server-Push

I can find only two documents that suggests randomization of boundary in example. But both of them does not describe why randomize is needed.
   (1)Original Documentation of Netscape uses "--ThisRandomText".
      http://web.archive.org/web/20060424004246/wp.netscape.com/assist/net_sites/pushpull.html
   (2)O'Reilly Koala book (HTML: The Definitive Guide) uses "--SomeRandomText".
       (Latest edition is renamed to "HTML & XHTML : The Definitive Guide")

Comment 16

9 years ago
This page explains how x-mixed-replace is designed to be used. It's also makes it clear how a known boundary could

http://www.abiglime.com/webmaster/articles/cgi/032498.htm

It's not clear to me what an attacker would gain by using the known boundary. They already have the ability to send content. x-mixed-replace with a known boundary allows them to send two copies of the content and have the second replace the first. 

Perhaps the issue is a case in which the first copy of the content is "good" content from a victim, but an attacker manages to craft a URL so that the good content is replaced with second piece of content which is malicious?

Having a clearer picture of how an exploit might would be helpful.
Assignee

Comment 17

9 years ago
it's a variation on 'http response splitting', which can be used to perform cross-site scripting attacks, cross-user defacement, web cache poisoning, and similar exploits.

http://cwe.mitre.org/data/definitions/113.html
Reporter

Comment 18

9 years ago
I think there are two patterns of attack for this bug.

1. set "--------- =_aaaaaaaaaa0" to query parameter. (for example, comment #8)

2. set "--------- =_aaaaaaaaaa0" to description of bug.
   I did not test, because it is too dangerous to test at here. It may fail.
Reporter

Comment 20

9 years ago
(In reply to comment #19)
> Attackers can inject ANY HTML tags code using x-mac-arabic
> 
And UTF-7, x-max-farasi and x-mac-hebrew can be used too (see bug 601429).
Reporter

Comment 21

9 years ago
(In reply to comment #18)
> I think there are two patterns of attack for this bug.
> 
> 2. set "--------- =_aaaaaaaaaa0" to description of bug.
>    I did not test, because it is too dangerous to test at here. It may fail.

At bugzilla, CRLF in Summary of bug is not allowed, and CRLF will be converted to SPACE.
As a result of this convert, What attackers can do with bug description (Summary) seems to be only "breaking response".

Updated

9 years ago
Blocks: 606854
Wow, this problem actually goes back to the VERY FIRST CHECKIN EVER for Bugzilla, meaning that it was in Bugzilla 1.0, probably, even.
Version: 3.6.2 → 2.10

Updated

9 years ago
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval3.4?
Flags: approval3.4+
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/CGI.pm
Committed revision 7590.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/CGI.pm
Committed revision 7472.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/CGI.pm
Committed revision 7200.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.4/
modified Bugzilla/CGI.pm
Committed revision 6781.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.2/
modified Bugzilla/CGI.pm
Committed revision 6402.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Security advisory sent, unlocking bug.
Group: bugzilla-security
Re-locking so as to not 0-day CGI.pm.
Group: bugzilla-security
Reporter

Comment 26

9 years ago
Secuyity Advisory http://www.bugzilla.org/security/3.2.8/ seems to contain MOJIBAKE in Credits.
> Frédéric Buclin
(In reply to comment #26)
> Secuyity Advisory http://www.bugzilla.org/security/3.2.8/ seems to contain
> MOJIBAKE in Credits.
> > Frédéric Buclin

I fixed it. Thanks for catching that! (I saw this encoding problem in the security advisory, but I thought this was a problem with Bugzilla itself when displaying the advisory.)
Reporter

Comment 28

9 years ago
-> VERIFIED
Now boundary is randomized.
Status: RESOLVED → VERIFIED
The CGI.pm with the fix has been released, unlocking this bug.
Group: bugzilla-security

Comment 31

9 years ago
I have submitted a patch to CGI::Simple now, based on the CGI.pm fix.
(In reply to comment #31)
> I have submitted a patch to CGI::Simple now, based on the CGI.pm fix.

Mark, can you attach a copy of that patch (or email it to me : tcallawa@redhat.com) ?

Thanks.
(In reply to comment #32)
> (In reply to comment #31)
> > I have submitted a patch to CGI::Simple now, based on the CGI.pm fix.
> 
> Mark, can you attach a copy of that patch (or email it to me :
> tcallawa@redhat.com) ?

Never mind, found it here:
https://github.com/AndyA/CGI--Simple/commit/e4942b871a26c1317a175a91ebb7262eea59b380
You need to log in before you can comment on or make changes to this bug.