Closed Bug 995917 Opened 10 years ago Closed 10 years ago

"view-source:" protocol cannot be loaded from a XUL (chrome) document in a tab anymore, after the bug 624883

Categories

(Firefox :: Extension Compatibility, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID
Firefox 30

People

(Reporter: yuki, Assigned: bobowen)

References

Details

(Keywords: addon-compat, regression)

By the bug 624883, the "view-source:" protocol is allowed only for top-level frame. However, if I expect to load the "view source" window in a tab, it doesn't work anymore. We need a way to allow loading of view-source URIs into in-content, privileged documents.

Steps to reproduce:
1. Install "Source Viewer Tab" addon from
   http://piro.sakura.ne.jp/xul/xpi/nightly/
   (the last released version on AMO doesn't work on Nightly 31.01.)
2. Press Ctrl-U

Expected result:
"View source" window-like content is loaded in a new tab, and the source of the current tab is shown.

Actual result:
"View source" window-like content is loaded in a new tab, but it just says "The address wasn't understood".
Blocks: 624883
Keywords: regression
Direct link to test add-on: http://piro.sakura.ne.jp/xul/xpi/nightly/viewsourceintab.xpi

Bob, is this an exception that can be easily added? Is it worth supporting this edge case?
Flags: needinfo?(bobowencode)
Keywords: addon-compat
Target Milestone: --- → Firefox 30
(In reply to Jorge Villalobos [:jorgev] from comment #1)
> Direct link to test add-on:
> http://piro.sakura.ne.jp/xul/xpi/nightly/viewsourceintab.xpi
> 
> Bob, is this an exception that can be easily added? Is it worth supporting
> this edge case?

I'll take a look later today.
Assignee: nobody → bobowencode
(In reply to Jorge Villalobos [:jorgev] from comment #1)

> Bob, is this an exception that can be easily added?

I'm not sure it is, at least as far as I can see.

(In reply to YUKI "Piro" Hiroshi from comment #0)
> By the bug 624883, the "view-source:" protocol is allowed only for top-level
> frame. However, if I expect to load the "view source" window in a tab, it
> doesn't work anymore. We need a way to allow loading of view-source URIs
> into in-content, privileged documents.

So, I've looked at this in debug and when it is in nsDocShell::DoURILoad (where we do the check), this doesn't appear to be a privileged document.
The owner is null and the document principal is not the system principal.

I'm not sure what you could do about this as I've not had much experience with add-ons.

freddyb - what would your view be on this if this was a privileged document?
Flags: needinfo?(bobowencode) → needinfo?(fbraun)
The main reason to disallow the `view-source` protocol was to prevent spoofing/clickjacking attacks that confuse users. The scenario is usually that the source code of a known URI is included and the user is tricked into submitting parts of the source code as some kind of "fake captcha" which then leaks the session cookie or the csrftoken.

I think we could just allow `view-source` in tabs, couldn't we?
To be extra sure I'd like some other security folks to weigh in though :)
Flags: needinfo?(fbraun)
(In reply to Frederik Braun [:freddyb] from comment #4)

> I think we could just allow `view-source` in tabs, couldn't we?
> To be extra sure I'd like some other security folks to weigh in though :)

You can use the view-source scheme directly in a tab.
This add-on has some extra UI bits and loads the source into some sort of frame.
I have to admit that I find the add-on code a bit difficult to navigate ...

... Piro - in what is the view-source URI actually loaded?
Is it a dynamically added iframe?
Flags: needinfo?(yuki)
My addon has two different modes.

1) With UI. It loads a URI like "view-source-tab:https://bugzilla.mozilla.org/show_bug.cgi?id=995917" into a tab. The "view-source-tab" protocol is implemented by my addon itself, and it actually loads a chrome document "chrome://viewsourceintab/content/viewer.xul". It provides "View Source" window partially compatible UI - the menu bar, the location indicator, the status bar, and the inline frame (<xul:browser>). They are statically defined in the chrome document. This mode doesn't work on Nightly 31.0a1.

2) Without UI. It loads a "view-source:" URI directly into a tab. This mode still works on Nightly 31.0a1.

> So, I've looked at this in debug and when it is in nsDocShell::DoURILoad (where we do the check), this doesn't appear to be a privileged document.
> The owner is null and the document principal is not the system principal.

If it is not loaded as a privileged document, I've done a wrong way to load a chrome document into a tab via my custom protocol. Then I'm ready to update my codes.
Flags: needinfo?(yuki)
(In reply to YUKI "Piro" Hiroshi from comment #6)

> it actually loads a chrome document
> "chrome://viewsourceintab/content/viewer.xul". It provides "View Source"
> window partially compatible UI - the menu bar, the location indicator, the
> status bar, and the inline frame (<xul:browser>). They are statically
> defined in the chrome document. This mode doesn't work on Nightly 31.0a1.

Hi Piro - thanks for that information.

Well, after a fair bit of digging, I now know a lot more about add-ons.
This method of re-writing functions from other privileged JavaScript seems like it would be quite brittle.
Anyway, that's not the problem here.

> If it is not loaded as a privileged document, I've done a wrong way to load
> a chrome document into a tab via my custom protocol. Then I'm ready to
> update my codes.

I don't think it should be privileged.
The <browser> is actually defined inside chrome://global/content/viewSource.xul and is of type content, so that means it isn't privileged.
This is good from a security view-point.

However, I think I've found a solution to your problem.
If you add the following two lines, to the bottom of the initializeOnLoad function in viewSourceOverlay.js (just before the |return true;|):

// Set the docshell used to load the source to a frame type of Browser, otherwise the view-source URI will be blocked.
gBrowser.docShell.setIsBrowserInsideApp(Components.interfaces.nsIScriptSecurityManager.NO_APP_ID);


This means that the browser within the document is treated like a top level window and so is allowed to load view-source URIs.

Let me know how you get on.
Flags: needinfo?(yuki)
(In reply to Bob Owen (:bobowen) from comment #7)
> I don't think it should be privileged.
> The <browser> is actually defined inside
> chrome://global/content/viewSource.xul and is of type content, so that means
> it isn't privileged.
> This is good from a security view-point.

I'm sorry, I misunderstood the meaning of the term "privileged" on Firefox. Because my codes can use XPCOM components via XPConnect, I thought that my codes in my addon are "privileged".

> // Set the docshell used to load the source to a frame type of Browser,
> otherwise the view-source URI will be blocked.
> gBrowser.docShell.setIsBrowserInsideApp(Components.interfaces.
> nsIScriptSecurityManager.NO_APP_ID);

Thanks a lot, I didn't know such a technique. It seems good for me. Now my addon works again. For addon authors who implement in-content UIs, it is very useful magic.

So, should I close this bug as RESOLVED-INVALID?
Flags: needinfo?(yuki)
(In reply to YUKI "Piro" Hiroshi from comment #8)
> (In reply to Bob Owen (:bobowen) from comment #7)

> I'm sorry, I misunderstood the meaning of the term "privileged" on Firefox.
> Because my codes can use XPCOM components via XPConnect, I thought that my
> codes in my addon are "privileged".

Part of the add-on may well be privileged.
Which would explain why you can use XPCOM components.
The <browser> loading the view-source is not privileged.

> > // Set the docshell used to load the source to a frame type of Browser,
> > otherwise the view-source URI will be blocked.
> > gBrowser.docShell.setIsBrowserInsideApp(Components.interfaces.
> > nsIScriptSecurityManager.NO_APP_ID);
> 
> Thanks a lot, I didn't know such a technique. It seems good for me. Now my
> addon works again. For addon authors who implement in-content UIs, it is
> very useful magic.

Excellent, thanks for letting me know.

> So, should I close this bug as RESOLVED-INVALID?

I think that makes as much sense as any, I'll do it now.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.