The default bug view has changed. See this FAQ.
Bug 924802 (CVE-2013-1742)

[SECURITY] (XSS) "id" and "sortkey" are not sanitized when editing flag types if categoryAction-foo is set

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Administration
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Mateusz Goik, Assigned: Frédéric Buclin)

Tracking

({sec-critical, wsec-xss})

2.17.1
Bugzilla 4.0
sec-critical, wsec-xss
Bug Flags:
approval +
approval4.4 +
blocking4.4.1 +
approval4.2 +
blocking4.2.7 +
approval4.0 +
blocking4.0.11 +
sec-bounty +

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
PoC:

http://localhost/bugzilla/editflagtypes.cgi?action=insert&can_fully_edit=1&id="><script>alert(1)</script>&token=&target_type=bug&check_clusions=1&name=test1&description=test2&product=TestProduct&component=TestComponent&categoryAction-include=Include&sortkey=1&is_active=on&is_requestable=on&cc_list=&is_requesteeble=on&is_multiplicable=on&grant_group=&request_group=

http://localhost/bugzilla/editflagtypes.cgi?action=insert&can_fully_edit=1&id=&token=&target_type=bug&check_clusions=1&name=test&description=test2&product=TestProduct&component=TestComponent&categoryAction-include=Include&sortkey=1"><script>alert(2)</script>&is_active=on&is_requestable=on&cc_list=&is_requesteeble=on&is_multiplicable=on&grant_group=&request_group=
The problem is here, starting on line 108 of editflagtypes.cgi:

    my $type = {};
    $type->{$_} = $cgi->param($_) foreach $cgi->param();
...
    $vars->{'type'} = $type;

We don't require the filtering of ".id" in templates in our XSS-checking test because we assume it will always be a property of an object which Bugzilla has validated the creation of. Here, we instead make up a fake object, trusting all the parameters on the URL. This is bad.

This code was added by LpSolit in bug 523205, and checked into the trunk (only) in October 2010. It is not present in 4.0, but is present in 4.2 and 4.4.

Gerv
Flags: sec-bounty?
(Assignee)

Comment 2

4 years ago
(In reply to Gervase Markham [:gerv] from comment #1)
> This code was added by LpSolit in bug 523205

Not true. I didn't write this code. I only moved the code out of a subroutine when refactoring editflagtypes.cgi, but the code was already there since flags have been implemented in Bugzilla 2.18, see bug 98801. I can reproduce with 4.0 and 3.6 (didn't try with older releases, but the code didn't change).

I pasted in the URL field a reduced testcase.
Summary: XSS - editflagtypes.cgi → [SECURITY] XSS in editflagtypes.cgi
Target Milestone: --- → Bugzilla 4.0
Version: unspecified → 2.18
(In reply to Frédéric Buclin from comment #2)
> (In reply to Gervase Markham [:gerv] from comment #1)
> > This code was added by LpSolit in bug 523205
> 
> Not true. 

Ah, my apologies.

Anyway, assuming the admin is logged in (if he's not, the PoC fails), if he can be persuaded to load this URL then his cookies can be stolen. :-|

Should we eliminate all exceptions to the filtering requirements? Or would that be overkill?

Gerv
(Assignee)

Comment 4

4 years ago
Neither "id" nor "sortkey" are sanitized or HTML-escaped when categoryAction-foo is set (it doesn't need to be a legal name as long as it starts with "categoryAction-"). So this URL works too (I replaced id by sortkey):

editflagtypes.cgi?sortkey="><script>alert(1)<%2Fscript>&categoryAction-foo=1
Assignee: administration → LpSolit
Status: NEW → ASSIGNED
Flags: blocking4.4.1+
Flags: blocking4.2.7+
Flags: blocking4.0.11+
Summary: [SECURITY] XSS in editflagtypes.cgi → [SECURITY] (XSS) "id" and "sortkey" are not sanitized when editing flag types if categoryAction-foo is set
(Assignee)

Comment 5

4 years ago
(In reply to Gervase Markham [:gerv] from comment #3)
> can be persuaded to load this URL then his cookies can be stolen. :-|

Do not forget that credentials cannot be stolen due to the HttpOnly attribute set on cookies.
(Assignee)

Comment 6

4 years ago
Created attachment 814861 [details] [diff] [review]
patch for trunk, v1
Attachment #814861 - Flags: review?(dkl)
(Assignee)

Comment 7

4 years ago
Created attachment 814867 [details] [diff] [review]
patch for 4.2 and 4.4, v1
Attachment #814867 - Flags: review?(dkl)
Keywords: wsec-xss
(Assignee)

Comment 8

4 years ago
Created attachment 814872 [details] [diff] [review]
patch for 4.0

Things are worse in 4.0. "action" and "target_type" weren't HTML-escaped either.
Attachment #814872 - Flags: review?(dkl)
Comment on attachment 814861 [details] [diff] [review]
patch for trunk, v1

Review of attachment 814861 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #814861 - Flags: review?(dkl) → review+
Comment on attachment 814867 [details] [diff] [review]
patch for 4.2 and 4.4, v1

Review of attachment 814867 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #814867 - Flags: review?(dkl) → review+
Comment on attachment 814872 [details] [diff] [review]
patch for 4.0

Review of attachment 814872 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #814872 - Flags: review?(dkl) → review+

Updated

4 years ago
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?
Assigning CVE-2013-1742 to this bug
Alias: CVE-2013-1742
(Assignee)

Updated

4 years ago
Blocks: 912643
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
(Assignee)

Updated

4 years ago
Version: 2.18 → 2.17.1
(Assignee)

Comment 13

4 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/filterexceptions.pl
modified template/en/default/admin/flag-type/edit.html.tmpl
Committed revision 8779.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified template/en/default/filterexceptions.pl
modified template/en/default/admin/flag-type/edit.html.tmpl
Committed revision 8625.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified template/en/default/filterexceptions.pl
modified template/en/default/admin/flag-type/edit.html.tmpl
Committed revision 8233.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified template/en/default/filterexceptions.pl
modified template/en/default/admin/flag-type/edit.html.tmpl
Committed revision 7761.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Group: bugzilla-security
(Assignee)

Comment 14

4 years ago
Security advisory sent.
Flags: sec-bounty? → sec-bounty+
Keywords: sec-critical
You need to log in before you can comment on or make changes to this bug.