Open Bug 1324383 Opened 7 years ago Updated 2 years ago

Stop honouring CSP for view-source

Categories

(Toolkit :: View Source, defect, P3)

Product:
Toolkit
Component:
View Source
Version:
53 Branch
Type:
defect

Tracking

()

People

(Reporter: u424271, Unassigned)

References

Details

Attachments

(1 file)

screenshot from page inspector
51.55 KB, image/jpeg
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20161218030213

Steps to reproduce:

Visit website with a CSP that blocks inline style [e.g. default-src 'self';] 
View page source


Actual results:

A CSP violation is caused by the following line in the source viewer:

<body id="viewsource" class="highlight" style="-moz-tab-size: 4" contextmenu="actions">


Expected results:

Style should have been embedded into
resource://gre-resources/viewsource.css
Component: Untriaged → DOM: Security
Product: Firefox → Core
But that inline style is the browser's style, not the page style. Why would the page CSP apply to the browser looking at the source of the page and not the rendered page itself?

Maybe it would be cleaner to embed the style into the resource as suggested, but I don't see how this is a "CSP Violation".
Flags: needinfo?(j162011)
I should have included the message shown in the console:

Content Security Policy: The page's settings observed the loading of a resource at self ("default-src view-source://"). A CSP report is being sent. Source: -moz-tab-size: 4.

It appears the page CSP is being applied to view-source://

Either the browser should:
a) not use inline styles in its page [OR]
b) not consider its internal styles a violation of CSP
Flags: needinfo?(j162011)
This would be a "DOM: Security" bug if we're somehow inheriting the page's CSP into view-source:

This is more likely a DevTools/ViewSource bug in either causing that inheritance, or injecting a CSP of their own for safety reasons (a good idea) without fully fixing the view-source "source".
Status: UNCONFIRMED → NEW
Component: DOM: Security → Developer Tools
Ever confirmed: true
Product: Core → Firefox
Component: Developer Tools → View Source
Product: Firefox → Toolkit
Hmm, I don't know enough details about how we choose the CSP to use for a URL.  I can't see any reason we'd want the page's CSP to be applied to the view source markup, since it's all browser generated.

:ckerschb, you've helped with other CSP issues in past, any idea how we could avoid using the page CSP in view source?
Flags: needinfo?(ckerschb)
I tried to debug that and it seems to me that we are facing the exact problem as in Bug 1326146. Whether this is a view:source or not does not matter in my opinion. It's simply the fact that we should not call nsStyleUtil::CSPAllowsInlineStyle() for browser code, as Ehsan explained within [1].

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1326146#c5
Flags: needinfo?(ckerschb)
See Also: → 1326146
FWIW, it seems that the style in question originates within nsHTML5ViewSourceUtils.cpp [1]. Ehsan, I suppose we would have to set some special attribute which we then somehow (how?) query within nsStyledElement::ParseStyleAttribute() to avoid calling CSP, right?

If you could provide some pointers on how to set up such a special attribute I think we should try to fix that - thanks for your help.

https://dxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5ViewSourceUtils.cpp#32
Flags: needinfo?(ehsan)
The problem here is a bit worse than bug 1326146, in that it's not just the style attribute we need to ignore, but also resource://gre-resources/viewsource.css.  Basically we need to ignore CSP for view-source documents.

In bug 561051, we explicitly made viewsource documents follow the CSP of their innermost URIs.  Here, we basically want to ignore the CSP of the innermost URI.  These two things can't really be reconciled together.

Dan, do we really want the CSP for https://x.com apply to viewsource:https://x.com?
Flags: needinfo?(ehsan) → needinfo?(dveditz)
CSP makes no sense for a view-source document -- it's the browser's document, not the web site's. It's supposed to be the moral equivalent of harmless plain text: no resource inclusion, no scripts, no frames, no embeds, etc. In reality we've converted it to an HTML document to make it pretty which does raise some worries CSP might solve (could we misinterpret the source and end up injecting HTML into _our_ document? Would scripts run?) but in that case we'd want it to be the _browser's_ CSP, not the document's.

the solution to bug 561051 wasn't the right thing to do. I suppose it was expedient to grab frame-source, and I think view-source was more plaint text back then so we must have figured the rest of the CSP wasn't doing anything anyway. But in the end we decided to kill view-source frames in bug 624883 which made bug 561051 unnecessary. (We also killed view-source in object/embed in bug 973837, and then eventually made it completely unlinkable by web content in bug 1172165.

(In reply to :Ehsan Akhgari from comment #7)
> we need to ignore, but also resource://gre-resources/viewsource.css.

Interesting: CSP should be giving a free pass to resource:// and chrome:// urls.

> Basically we need to ignore CSP for view-source documents.

Yes. Given later bug fixes we can undo bug 561051. It might be worth attaching our own CSP as a back-up (for example, "default-src: 'none'; style-src: 'unsafe-inline';"), but hopefully the view-source docshell has disabled javascript and plugins anyway. I'm assuming you're wrong that viewsource.css is getting blocked -- I still see styles when viewing source on sites with CSP.
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #8)
> CSP makes no sense for a view-source document -- it's the browser's
> document, not the web site's. It's supposed to be the moral equivalent of
> harmless plain text: no resource inclusion, no scripts, no frames, no
> embeds, etc. In reality we've converted it to an HTML document to make it
> pretty which does raise some worries CSP might solve (could we misinterpret
> the source and end up injecting HTML into _our_ document? Would scripts
> run?) but in that case we'd want it to be the _browser's_ CSP, not the
> document's.

This all makes sense to me.  One question though, what do you mean by browser's CSP?  the example you gave below?

> the solution to bug 561051 wasn't the right thing to do. I suppose it was
> expedient to grab frame-source, and I think view-source was more plaint text
> back then so we must have figured the rest of the CSP wasn't doing anything
> anyway. But in the end we decided to kill view-source frames in bug 624883
> which made bug 561051 unnecessary. (We also killed view-source in
> object/embed in bug 973837, and then eventually made it completely
> unlinkable by web content in bug 1172165.

OK, adjusting the summary.

> (In reply to :Ehsan Akhgari from comment #7)
> > we need to ignore, but also resource://gre-resources/viewsource.css.
> 
> Interesting: CSP should be giving a free pass to resource:// and chrome://
> urls.

Indeed, subjectToCSP() does that.

> > Basically we need to ignore CSP for view-source documents.
> 
> Yes. Given later bug fixes we can undo bug 561051. It might be worth
> attaching our own CSP as a back-up (for example, "default-src: 'none';
> style-src: 'unsafe-inline';"), but hopefully the view-source docshell has
> disabled javascript and plugins anyway.

Based on a quick test, javascript is definitely not disabled, and I doubt anything else is too.

> I'm assuming you're wrong that
> viewsource.css is getting blocked -- I still see styles when viewing source
> on sites with CSP.

I was indeed wrong.  I was confused due to bug 1333617.
Summary: Inline style in view-source can cause CSP violation → Stop honouring CSP for view-source
Moved to the backlog (P3) since nobody is working on it now.
Honza
Priority: P2 → P3
That should become way simpler once Bug 965637 has landed.
Depends on: 965637
It would be nice to have userscripts be able to run Javascript on view-source pages, if at all compatible with other security concerns. Would this be possible if the CSP is changed as per this bug? The same applies to extensions, I think (or do they already have this ability?).

For example, I use a userscript that automatically highlights search terms (from Google) in a tab opened by clicking on a link from the Google results page. It would be nice to have this be possible in view-source pages as well. The userscript can also highlight several different words or phrases in any document by selecting them and adding them to the overlay, which is very practical.

And I can think of many other use-cases in which this would be nice to have. Another example would be a userscript or extension that automatically prepends a list of all urls ending on ".mp4" at the top of any view-source tab, so the user can easily download or preview media files. Etc.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: