Last Comment Bug 621572 - (CVE-2010-4572) [SECURITY] chart.cgi vulnerable to header-injection due to use of |print "Location:"| instead of $cgi->redirect
(CVE-2010-4572)
: [SECURITY] chart.cgi vulnerable to header-injection due to use of |print "Loc...
Status: RESOLVED FIXED
[infrasec:xss][ws:critical]
:
Product: Bugzilla
Classification: Server Software
Component: Reporting/Charting (show other bugs)
: 4.1
: All All
: P1 critical (vote)
: Bugzilla 3.2
Assigned To: Reed Loden [:reed] (use needinfo?)
: default-qa
Mentors:
chart.cgi?cmd-0=1&format=table&query_...
Depends on: CVE-2010-2761
Blocks: 620540
  Show dependency treegraph
 
Reported: 2010-12-27 12:02 PST by Frédéric Buclin
Modified: 2011-08-28 05:28 PDT (History)
5 users (show)
LpSolit: approval+
LpSolit: approval4.0+
mkanat: blocking4.0+
LpSolit: approval3.6+
mkanat: blocking3.6.4+
LpSolit: approval3.4+
mkanat: blocking3.4.10+
LpSolit: approval3.2+
mkanat: blocking3.2.10+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (962 bytes, patch)
2010-12-27 22:43 PST, Reed Loden [:reed] (use needinfo?)
mkanat: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2010-12-27 12:02:30 PST
Found in chart.cgi:

    print "Location: query.cgi?format=" . $cgi->param('query_format') .
                                          ($params ? "&$params" : "") . "\n\n";

    print "Location: buglist.cgi" . ($params ? "?$params" : "") . "\n\n";

We should use $cgi->redirect instead.
Comment 1 Reed Loden [:reed] (use needinfo?) 2010-12-27 22:09:19 PST
This is another bug similar to bug 591165. Due to the use of Location:, header-injection is easily possible.
Comment 2 Reed Loden [:reed] (use needinfo?) 2010-12-27 22:43:30 PST
Created attachment 499986 [details] [diff] [review]
patch - v1

This should work, but should I be using |-uri => correct_urlbase() . "blah.cgi"| instead, or is this fine? We seem to vary on usage one way or another throughout the codebase.
Comment 3 Daniel Veditz [:dveditz] 2010-12-27 23:00:21 PST
call this one CVE-2010-4572
Comment 4 Frédéric Buclin 2010-12-28 03:52:02 PST
(In reply to comment #2)
> This should work, but should I be using |-uri => correct_urlbase() .
> "blah.cgi"| instead, or is this fine? We seem to vary on usage one way or
> another throughout the codebase.

http://search.cpan.org/~lds/CGI.pm-3.50/lib/CGI.pm#GENERATING_A_REDIRECTION_HEADER recommends to use full URL, not relative ones:

"You should always use full URLs (including the http: or ftp: part) in redirection requests. Relative URLs will not work correctly."
Comment 5 Max Kanat-Alexander 2010-12-28 12:52:01 PST
For now you should be using $cgi->url to generate a full URL, unless $cgi->redirect does that internally.
Comment 6 Max Kanat-Alexander 2010-12-30 15:28:50 PST
Comment on attachment 499986 [details] [diff] [review]
patch - v1

Okay, this is actually what buglist.cgi does already, so this is fine.
Comment 7 Reed Loden [:reed] (use needinfo?) 2011-01-24 10:16:15 PST
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified chart.cgi
Committed revision 7673.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified chart.cgi
Committed revision 7530.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified chart.cgi
Committed revision 7223.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.4/
modified chart.cgi
Committed revision 6790.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.2/
modified chart.cgi
Committed revision 6411.
Comment 8 Frédéric Buclin 2011-01-24 17:20:06 PST
Security advisory sent. Removing the security flag.
Comment 9 Mario Gomes 2011-08-28 05:28:20 PDT
test.

Note You need to log in before you can comment on or make changes to this bug.