nsScriptSecurityManager::GetBaseURIScheme doesn't handle jar:view-source:

RESOLVED FIXED in mozilla1.8beta2

Status

()

Core
Security
P1
critical
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({fixed-aviary1.0.4, fixed1.7.8})

Trunk
mozilla1.8beta2
fixed-aviary1.0.4, fixed1.7.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(2 attachments)

(Assignee)

Description

12 years ago
nsScriptSecurityManager::GetBaseURIScheme doesn't handle jar:view-source:...
correctly because the jar: and view-source: cases don't use recursion as they
ought to.  This provides a way to get around
nsIScriptSecurityManager.DISALLOW_JAVASCRIPT or DISALLOW_JAVASCRIPT_OR_DATA checks.

Testcase to be attached, variant of bug 290036 and bug 290949.
(Assignee)

Comment 1

12 years ago
Created attachment 183196 [details]
testcase
(Assignee)

Comment 2

12 years ago
Created attachment 183198 [details] [diff] [review]
patch (use recursion)
Attachment #183198 - Flags: superreview?(dveditz)
Attachment #183198 - Flags: review?(caillon)
Attachment #183198 - Flags: approval1.8b2?
Attachment #183198 - Flags: approval1.7.8?
Attachment #183198 - Flags: approval-aviary1.0.4?
(Assignee)

Updated

12 years ago
Flags: blocking1.8b2?
Flags: blocking1.7.8?
Flags: blocking-aviary1.0.4?
Comment on attachment 183198 [details] [diff] [review]
patch (use recursion)

>Index: nsScriptSecurityManager.cpp

>-    while(PL_strcmp(scheme.get(), "view-source") == 0)
>+    if (PL_strcmp(scheme.get(), "view-source") == 0)

Since you're touching this, we should use libc's version of strcmp(), not the
PL version.

...

>-    //-- if uri is an about uri, distinguish 'safe' and 'unsafe' about URIs
>+    //-- if aURI is an about uri, distinguish 'safe' and 'unsafe' about URIs
>     static const char aboutScheme[] = "about";
>     if(nsCRT::strcasecmp(scheme.get(), aboutScheme) == 0)

The scheme is be normalized to all lowercase.  (Do we ever get schemes that
necko specifically knows about that are not lowercased?).  We can change this
to the libc strcmp() version.

r=caillon
Attachment #183198 - Flags: review?(caillon) → review+
(Assignee)

Comment 4

12 years ago
My goal here was to change as little as possible; it's already using
EqualsLiteral on the trunk.

Comment 5

12 years ago
<http://test.bclary.com/tests/mozilla.org/security/293671-01.html> version of
the testcase without code execution.
Comment on attachment 183198 [details] [diff] [review]
patch (use recursion)

sr=dveditz
Attachment #183198 - Flags: superreview?(dveditz) → superreview+

Comment 7

12 years ago
Comment on attachment 183198 [details] [diff] [review]
patch (use recursion)

a=asa
Attachment #183198 - Flags: approval1.8b2?
Attachment #183198 - Flags: approval1.8b2+
Attachment #183198 - Flags: approval1.7.8?
Attachment #183198 - Flags: approval1.7.8+
Attachment #183198 - Flags: approval-aviary1.0.4?
Attachment #183198 - Flags: approval-aviary1.0.4+
(Assignee)

Updated

12 years ago
Severity: normal → critical
Status: NEW → ASSIGNED
Keywords: fixed-aviary1.0.4, fixed1.7.8
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.8beta2
(Assignee)

Comment 8

12 years ago
Checked in to trunk (still matters for data:).
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: blocking1.8b2?
Flags: blocking1.7.8?
Flags: blocking-aviary1.0.4?
Clearing security flag from announced vulnerabilities fixed in Firefox
1.0.4/Mozilla 1.7.8
Group: security
You need to log in before you can comment on or make changes to this bug.