Closed Bug 293671 Opened 20 years ago Closed 20 years ago

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

Categories

(Core :: Security, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: dbaron, Assigned: dbaron)

Details

(Keywords: fixed-aviary1.0.4, fixed1.7.8, Whiteboard: [patch])

Attachments

(2 files)

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.
Attached file testcase (deleted) —
Attached patch patch (use recursion) (deleted) — Splinter Review
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?
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+
My goal here was to change as little as possible; it's already using EqualsLiteral on the trunk.
<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 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+
Severity: normal → critical
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.8beta2
Checked in to trunk (still matters for data:).
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: