600464 - (CVE-2010-3172) [SECURITY] Content/Header injection due to non-random multipart/x-mixed-replace boundary

Status: VERIFIED FIXED

(Bugzilla :: Query/Bug List, defect, P1)

Description

[Masahiro YAMADA]

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

Comment 1

[Masahiro YAMADA]

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.

Comment 2

[Reed Loden [:reed]]

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;
}

Comment 3

[:glob ✱]

Attachment: randomise mime boundary.1

we already overwrite multipart_init, so the fix is trivial.

Comment 4

[:glob ✱]

i have emailed the maintainers of CGI.pm to make them aware of this issue.

Comment 5

[Gervase Markham [:gerv]]

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

[Frédéric Buclin]

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.

Comment 7

[:glob ✱]

Attachment: randomise mime boundary.2

Comment 8

[:glob ✱]

here's a slightly clearer example url ..

https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc=---------%20%3D_aaaaaaaaaa0%0A%0Dcontent-type:%20text/html%0A%0Dcontent-length:%2016%0A%0D%0A%0Dhello%20world.

Comment 9

[Dave Miller [:justdave]]

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

Comment 10

[Dave Miller [:justdave]]

s/they're/their/

Comment 11

[Dave Miller [:justdave]]

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

Comment 12

[Reed Loden [:reed]]

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

Comment 13

[Masahiro YAMADA]

CGI::Simple seems to have same problem.

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

Comment 14

[Masahiro YAMADA]

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 15

[Masahiro YAMADA]

Source code list that may have same problem:

http://www.google.com/codesearch?lr=&q=multipart_init\s*\%28+lang%3Aperl+file%3A[^C][^G][^I][^\.][^p][^m]%24

Comment 16

[Mark Stosberg]

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.

Comment 17

[:glob ✱]

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

Comment 18

[Masahiro YAMADA]

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.

Comment 19

[Masahiro YAMADA]

Attackers can inject ANY HTML tags code using x-mac-arabic

following example redirects serch result to http://www.mozilla.org

https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc=---------%20%3D_aaaaaaaaaa0%0A%0Dcontent-type:%20text/html%3Bcharset%3Dx-mac-arabic%0A%0D%0A%0D%BCscript%20%BE%0A%0Dlocation.href='http://www.mozilla.org'%3B//%BC/script%20%BE

Comment 20

[Masahiro YAMADA]

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

Comment 21

[Masahiro YAMADA]

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

Comment 22

[Max Kanat-Alexander]

Wow, this problem actually goes back to the VERY FIRST CHECKIN EVER for Bugzilla, meaning that it was in Bugzilla 1.0, probably, even.

Comment 23

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/CGI.pm
Committed revision 7590.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/CGI.pm
Committed revision 7472.

Committing to: bzr+ssh://[e-mail]/bugzilla/3.6/
modified Bugzilla/CGI.pm
Committed revision 7200.

Committing to: bzr+ssh://[e-mail]/bugzilla/3.4/
modified Bugzilla/CGI.pm
Committed revision 6781.

Committing to: bzr+ssh://[e-mail]/bugzilla/3.2/
modified Bugzilla/CGI.pm
Committed revision 6402.

Comment 24

[Max Kanat-Alexander]

Security advisory sent, unlocking bug.

Comment 25

[Max Kanat-Alexander]

Re-locking so as to not 0-day CGI.pm.

Comment 26

[Masahiro YAMADA]

Secuyity Advisory http://www.bugzilla.org/security/3.2.8/ seems to contain MOJIBAKE in Credits.
> Frédéric Buclin

Comment 27

[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.)

Comment 28

[Masahiro YAMADA]

-> VERIFIED
Now boundary is randomized.

Comment 29

[Reed Loden [:reed]]

CGI v3.50 released -- http://www.nntp.perl.org/group/perl.perl5.changes/2010/11/msg28043.html

Comment 30

[Max Kanat-Alexander]

The CGI.pm with the fix has been released, unlocking this bug.

Comment 31

[Mark Stosberg]

I have submitted a patch to CGI::Simple now, based on the CGI.pm fix.

Comment 32

[Tom "spot" Callaway]

(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 : [e-mail]) ?

Thanks.

Comment 33

[Tom "spot" Callaway]

(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 :
> [e-mail]) ?

Never mind, found it here:
https://github.com/AndyA/CGI--Simple/commit/e4942b871a26c1317a175a91ebb7262eea59b380