Closed Bug 387317 Opened 13 years ago Closed 13 years ago

[FIX]XBL resource stylesheet loads don't do security checks

Categories

(Core :: XBL, defect, major)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

(Whiteboard: [sg:low] post 1.8-branch)

Attachments

(1 file)

In particular, nsXBLResourceLoader::LoadResources doesn't call CheckLoadURI and doesn't call content policies.
Flags: blocking1.9?
Er, we do this for images.  Just not for stylesheets.  I would say we should just make the CSSLoader do it in this case.
Summary: XBL resource stylesheet and image loads don't do security checks → XBL resource stylesheet loads don't do security checks
Attached patch Proposed fixSplinter Review
Peter, can you take a look?  The XBL part is pretty tame, really... if you can't, let me know and I'll ping sicking.
Attachment #271484 - Flags: superreview?
Attachment #271484 - Flags: review?
Attachment #271484 - Flags: superreview?(peterv)
Attachment #271484 - Flags: superreview?
Attachment #271484 - Flags: review?(peterv)
Attachment #271484 - Flags: review?
The trunk patch also fixes XUL prototype stylesheet loads.

On branch the XUL prototype thing seems to happen only for chrome sheets, so it's only XBL that needs to be fixed.  I'll probably do it via copy/paste there...
Assignee: nobody → bzbarsky
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Summary: XBL resource stylesheet loads don't do security checks → [FIX]XBL resource stylesheet loads don't do security checks
Ah, and on branch this code uses LoadStyleLink, which does a security check.  So branch is completely safe.
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Whiteboard: trunk-only
Comment on attachment 271484 [details] [diff] [review]
Proposed fix

>@@ -3547,17 +3547,17 @@ nsHTMLEditor::ReplaceStyleSheet(const ns
>   if (!ps) return NS_ERROR_NOT_INITIALIZED;
>   nsIDocument *document = ps->GetDocument();
>   if (!document)     return NS_ERROR_NULL_POINTER;
> 
>   nsCOMPtr<nsIURI> uaURI;
>   rv = NS_NewURI(getter_AddRefs(uaURI), aURL);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  rv = cssLoader->LoadSheet(uaURI, this);
>+  rv = cssLoader->LoadSheet(uaURI, nsnull, nsnull, this);

I wonder if this could be triggered from desginmode or contenteditable, we might want to do the checks then. Anyway, different bug :-).
Attachment #271484 - Flags: superreview?(peterv)
Attachment #271484 - Flags: superreview+
Attachment #271484 - Flags: review?(peterv)
Attachment #271484 - Flags: review+
Checked in.  We could use tests for this, but not sure how to write them....

Removing security flag, since this is trunk-only.
Group: security
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: blocking1.9? → in-testsuite?
Resolution: --- → FIXED
Whiteboard: trunk-only → [sg:low] post 1.8-branch
Flags: wanted1.8.1.x-
You need to log in before you can comment on or make changes to this bug.