sidebarURLSecurityCheck should use nsIScriptSecurityManager.idl

NEW
Unassigned

Status

19 years ago
8 years ago

People

(Reporter: nicolas.chauveau, Unassigned)

Tracking

({arch})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 obsolete attachment)

(Reporter)

Description

19 years ago
A html file located on C:\ can't be opened on the sidebar.
The javascript opens the popup window.
fter the confirmation the loading animation never stop.

It is not because of a bad content :
I have tried with the example page (Developer's Guide) cut and paste.
It doesn't work anymore.

I have tried with a real page on a web site ("http:" URL) it works fine.
nicolas.chauveau@ferri.fr - we need a bit more information to proceed here.

As far as I am aware, the sidebar has no mechanism for opening URLs. Could you 
please describe exactly what you are doing, and in which build of Mozilla? 
Thanks :-)

Gerv
(Reporter)

Comment 2

19 years ago
To open new panels in the sidebar I use Netscape's "My sidebar developer's guide".

The html to add a panel :
<HTML>
<HEAD>
<TITLE>TEST</TITLE>
<script language="JavaScript">
function testfunction()
{ if ((typeof window.sidebar == "object") &&
      (typeof window.sidebar.addPanel == "function"))
 { window.sidebar.addPanel ("Ferri","file:///C:/fol.html",""); } }
</script>
</HEAD>
<BODY>

<p>
<a href="JavaScript:testfunction();"><img src="AddTabNS6_fr.gif" border=0
height=45 width=100></a>
<p>

<p>
<a href="JavaScript:testfunction();">add panel</a>
<p>

<p>
<a href="file:///C:/fol.html">test</a>
<p>

</BODY>
</HTML>



The html page in the panel :
<html>
<head>
<title>My Sidebar Developer's Doc Tab</title>
</head>
<body bgcolor="#ffffee"><font face="sans-serif,Arial,Helvetica" size="1">
<h2>My Sidebar Developer's Guide</h2>
The sections in this document include:
<ul TYPE="circle">
<li><a target='_content' href="../sidebardev.html#intro">Introduction to My
Sidebar</a>
<li><a target='_content' href="../sidebardev.html#example">Example My Sidebar
Tab</a></li>
<li><a target='_content' href="../sidebardev.html#developing">Developing My
Sidebar Tabs</a></li>
<li><a target='_content' href="../sidebardev.html#creating">Creating a Simple My
Sidebar Tab</a></li>
<li><a target='_content' href="../sidebardev.html#adding">Adding a Tab to My
Sidebar</a></li>
</ul>
</font>
</body>
</html>


The script don't work when the page is a local file (C:/fol.html in my example)
but it works fine with a distant URL (http://...)

Whith a local file, after the "Add panel to my sidebar" confirmation window all
seems to be correct but when the new panel is selected to be active the
"loading" animation runs and never stops.
Well, if I check the value of typeof window.sidebar in the function 
testfunction, it comes up as undefined, not "object"... So, obviously the
add panel code isn't getting run. I'm not quite sure why NS would
recommend you use dodgy code... <shrug>

Gerv

Comment 4

19 years ago
The JS is fine, I use it for mine right now, i'll take a look at this tomorrow.
 Reassigning to me.  I have a feeling that file:// is not a supported protocol,
but I may have a way around it.
Assignee: slamm → kerz

Comment 5

18 years ago
Marking as NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

18 years ago
can you try this again and see if it works, not sure if this is an issue or not.
Assignee: kerz → matt

Updated

18 years ago
Keywords: nsbeta1

Comment 7

18 years ago
I almost filed a duplicate.

This still is an issue in M0.7:
   window.sidebar.addPanel("bla", "http://www.some.url", "");
works, but
   window.sidebar.addPanel("bla", "c:\some\local\address", "");
doesn't. Nor does
   window.sidebar.addPanel("bla", "file:///c|/local/address", "");

If you do any of the latter, the JavaScript console shows:

[Exception... "Script attempted to add sidebar panel from illegal source
[ns|Sidebar::addPanel]" etc.

Comment 8

18 years ago
I don't belive this to be a bug.  You can't be opening local files from the
sidebar, as that is a security risk.  I know that makes it hard to create them
without a website, but who is to stop someone from going into your harddrive and
seeing stuff they shouldn't, or causing general mischief.  I'll leave it for
matt to mark wontfix.

Comment 9

18 years ago
that's a security feature, i've been meaning to alter its functionality 
slightly, however web urls will _never_ be allowed to do this.
Keywords: nsbeta1 → nsbeta1-
*** Bug 57219 has been marked as a duplicate of this bug. ***
*** Bug 57219 has been marked as a duplicate of this bug. ***

Comment 12

18 years ago
I don't see the security hazard. I guess there is no way for the webpage that 
set the sidebar to access the sidebar's content if the sidebar's URL is from a 
different domain (and http://www.blabla.com and file:///bla/ certainly count as 
different domains)?

At any rate, local sidebars are IMHO by far the easiest way to customize 
Mozilla. For example, I would like to create a sidebar that displays the DOM 
tree of the main window's document. To do this I have to use privileged scripts 
which, unless I want to pay a lot of money to get them signed, I can only run on 
file:// protocol. 

It would be alright though if at least local pages could set local sidebars. 
Even though that would make it rather complicated for me to distribute my 
privileged sidebars to others...

Comment 13

18 years ago
*** Bug 66200 has been marked as a duplicate of this bug. ***
you can also use chrome:// urls for such scripts and since they are part of
chrome they should be able to install themselves.  drbrain@segment7.net may be
able to help you with that a bit, since he's the author of a sidebar that needs
chrome privileges.

Comment 15

18 years ago
I understand your problem and am going to work on it.
steps taken:
* taking bug
* setting milestone
* rewriting summary to describe my intentions
* cc people responsible for current code.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/components/sidebar/src
/nsSidebar.js#91
1.17 <mstoltz@netscape.com> 31 Oct 2000 16:48
 Bug 58021 r=mccabe sr=hyatt. Added security check to sidebar.addPanel to 
prevent js/chrome insertion

This is the only information that you are likely to find on the problem.

I think that the interface we should be using lives in 
http://lxr.mozilla.org/seamonkey/source/caps/idl/nsIScriptSecurityManager.idl

As this is my bug, I have a few rules. Please excuse my rules, but other bugs 
got into big fights over this nonsense, so this is my attempt to avoid that.
a) I'd like to keep comments in this bug restricted to the _technical_ aspect 
of the bug and reduce the project management and advocacy spam as much as 
possible so if you have non technical questions, etc, find me on IRC
b) don't apply netscape management rules to my bugs
c) don't change the scope or purpose of my bugs.
This means that after this fix is committed (if it ever gets committed), anyone 
who finds a flaw in it may feel free to assign a new bug to me as caused by 
this bug (so this bug would block your new bug).

This is standard procedure, but I feel the need to spell it out as other people 
have broken similarly obvious guidelines.

A few important notes:
a) I just took this bug.
b) I intend to work on it.
c) Bugzilla is an open system, however security bugs can be and often are 
closed.
d) Mozilla is an open project. Once a bug is reported in bugzilla and someone 
says that they will fix it, the bug is theirs.
Assignee: matt → timeless
Keywords: nsbeta1- → arch
OS: Windows NT → All
Hardware: PC → All
Summary: The sidebar can't open local file URL → sidebarURLSecurityCheck should use nsIScriptSecurityManager.idl
Target Milestone: --- → mozilla1.0

Comment 16

18 years ago
spam : changing qa to sujay (New Sidebar QA)
QA Contact: shrir → sujay
As the security manager is my module, I have a few rules. Actually just one -
don't check in any changes to security checks without my review.

I've opened bug 58021 to the public so you can read my rationale for fixing it
the way I did. The reasons I wrote a quick security check function in JS instead
of calling nsIScriptSecurityManager are 1) that is not very JS-friendly, as it
requires nsIURI objects instead of url strings. I'm planning to fix this soon.
2) The semantics of this particular security check are different than the one in
the security manager (CheckLoadURI). In most situations, javascript: URLS are
just fine, but in the case of sidebar thay're dangerous. You're right that local
code should be able to add a local sidebar. If this doesn't work it's an
omission on my part.

I'm not clear on what you're complaining about WRT "netscape management rules,"
but I assume you meant our disclosure policy for security bugs. The only
security bugs marked NS-confidential are those filed by Netscape employees or
contractors. We do not and never did mark a bug as confidential if it came from
a source outside Netscape. We have no right to do that. If a bug is confidential
and you think it should not be, it's probably an oversight on my part. It
happens. Email me and I'll open it up.

Comment 18

18 years ago
See also bug 50575. There must be a mechanism for adding a privileged sidebar
panel which does not require extensive user intervention.

This whole class of problem would vanish (or become irrelevant) if XPIs could
use Components. Is there a bug explaining why they can't?
Is this still a bug? Is thare an actual loss of functionality here? I explained
why I wrote this security check the way I did. If this is causing problems,
let's address them. Otherwise, this bug should be marked invalid.

It looks like te original issue was not being able to open sidebars loaded
locally. Is this still a problem? I don't see why we would need to use
CheckLoadURI in this case (though maybe we should).

Comment 20

18 years ago
yes i still can't add a gopher url. is there a bug for making the 
nsIScriptSecurityManager.idl script friendly? if so could you please mark that 
bug as blocking this one?

[fwiw netscape management rules was to prevent people from marking nsbeta* 
because it doesn't apply to outsiders]

Comment 21

18 years ago
At least the part of the bug I'm worried about still exists in yesterday's 
build. The problem is that I cannot add a local page as a sidebar panel, neither 
from within file: nor from http:.

As I explained, this is a real loss of functionality.
At least for local pages it should be possible to add other local pages as 
sidebar panel. And it would be very nice indeed if even pages from http: could 
do so.

Comment 22

18 years ago
If the problem with using CheckLoadURI is that it always allows javascript:
URLs, how about making sidebarURLSecurityCheck call CheckLoadURI but also make
sure the URL isn't a javascript: URL?

Comment 23

18 years ago
This week I found still the JS security check in the CVS and thought about the 
following change:

function sidebarURLSecurityCheck(url, win)
{
    var re = new RegExp("(^chrome://[^/]+/content/)","");
    var res = re.exec(window.location.href);

    // url is part of the same package as script source
    if (res && url.substring(0, res[1].length) == res[1])
        return;

    if (url.search(/(^http:|^ftp:|^https:)/) == -1)
        throw "Script attempted to add sidebar panel from illegal source";
}

This would allow 3rd party add-on authors to add something to the sidebar (of 
course I'd love to see sidebar access from XPI).

In case of chrome URLs there are other issues to mind, eg what happens if the 
package is removed (how?). See bug 77564.

Anyway, adding file and chrome URLs to the sidebar is an issue.

Comment 24

18 years ago
*** Bug 92703 has been marked as a duplicate of this bug. ***

Comment 25

18 years ago
Created attachment 43915 [details] [diff] [review]
proposed patch

Comment 26

18 years ago
I've attached a proposed patch, not sure if there are any security problems with
it. I'm not sure if I should get the "from" url from this.window.document.URL or
is there a better/more accurate/safer way?

Comment 27

18 years ago
basic: please change the |if(| to |if (|,

personally i wouldn't check for javascript: because the origin might be 
pythonscript: or perlscript: if someone added support for that ...

otherwise i like the patch. hopefully mstoltz will too.
Assignee: timeless → _basic
Keywords: approval, patch, review

Comment 28

18 years ago
ok timeless, I'll create another patch. But I've some concerns myself after
looking at nsScriptSecurityManager.cpp

By using nsScriptSecurityManager without providing additional checks we allow
the following protocols from pages on any protocol:
 "http", "https", "pop", "news", "javascript", "ftp", "about:blank", "about:"
 "about:mozilla", "about:credits", "mailto", "aim", "data", "gopher", "finger"
With a page on the "file" protocol we would allow the "file" protocol in
addition to the ones mentioned above.

I guess the danger is only "javascript" for now, as timeless mentioned there
will be other "script protocols" to worry about. Maybe we need a
nsScriptSecurityManager::DISALLOW_SCRIPT flag that would filter out all "script
protocols" ? To make this flexible there might also need to be prefs for
allowing and dissallowing protocols?

Another thing is "pop", "news", "mailto" and "aim" are useless in the sidebar as
all they do is cause MailNews (and aim in NS6) to be called. Should we filter
them out? Then again if any of these protocols are supported in the browser
someday... hey, it could happen, I'd love to see a html based newsgroup
interface. ;-)

Comment 29

18 years ago
err the above comment:

To make this flexible there might also need to be prefs for
allowing and dissallowing protocols?

should read:

To make this flexible there might also need to be prefs for
defining which protocols are considered scripts?

Comment 30

18 years ago
After looking at nsScriptSecurityManager.cpp again I think the patch I attached
should not be checked in. The problem is that checkLoadURI allows protocols not
listed in it to pass with just a warning. I think it needs to be rewritten to
*not* allow unlisted protocol to pass and allow newer protocols to be added via
prefs.
Eww, tabs.

Restricting new URI schemes unless they're pref'd in sounds good to me.

/be
Yeah, using prefs sounds like a good idea to me. I'll take this bug.
Assignee: _basic → mstoltz

Comment 33

18 years ago
The sidebar javascript code needs an unspoofable way to get the URL of the page 
trying to add the sidebar panel.  See also bug 87428 (link toolbar) and bug 
36946 (view source).
Less important bugs retargeted to 0.9.9
Target Milestone: mozilla1.0 → mozilla0.9.9
Target Milestone: mozilla0.9.9 → mozilla1.2
Noting that nsSidebar.prototype.addSearchEngine() has a similar limitation -
search icons and .src files are restricted to http:// URLs.  Again, this
prevents file:// or chrome:// from being used, both of which are useful,
particularly chrome://

I considered adding a new bug, but it seems to make sense for this bug to
incorporate the same check.  Thus, the blocks:

        if (engineURL.search(/^http:\/\//i) == -1)
        {
            debug ("must use HTTP to fetch search engine file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        if (iconURL.search(/^http:\/\//i) == -1)
        {
            debug ("must use HTTP to fetch search icon file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

should simply say:
      sidebarURLSecurityCheck(engineURL);
      sidebarURLSecurityCheck(iconURL);

and we get the fix for free.

[Also noting the keywords seem to imply the patch is waiting review and
approval, but the comments in this bug imply otherwise]

Comment 36

17 years ago
*** Bug 142275 has been marked as a duplicate of this bug. ***

Comment 37

17 years ago
I have similar problem, I want to add a sidebar panel from install script in my
xpi package. So the URL is chrome://...
But it's not possible with current security check function.
I also noticed that the DOM inspector pref panel has a work around which adds
its sidebar panel manually. It could use sidebar API and save footprint :)

Status: NEW → ASSIGNED
Target Milestone: mozilla1.2alpha → Future

Comment 38

17 years ago
There seem to be different (some hardcoded) security limitations in different
places. For example when adding a file: sidebar from a file: page, this change
must be made:
components\nsSideBar.js:93:
if (url.search(/(^http:|^ftp:|^https:|^file:)/) == -1)

Now the tab can be added, but the loading hangs. To make the file:-links
actually work:
defaults\prefs\all.js:600:
pref("security.checkloaduri", false);

Shouldn't file: pages load fine from an other file: page, even from the sidebar?
(Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1) Gecko/20020826)

Updated

16 years ago
Attachment #43915 - Attachment is obsolete: true

Comment 39

16 years ago
mstoltz: is this bug still valid? should it block bug 193255 (CheckLoadURI metabug?)
The fact that sidebars can't be data: URIs is unfortunate. It works fine in
Opera. See, e.g.: http://ln.hixie.ch/?start=1063743348&count=1
Just out of interest, why would a javascript: sidebar be a security risk?
Product: Browser → Seamonkey
Assignee: security-bugs → sidebar
Status: ASSIGNED → NEW
QA Contact: sujay
Assignee: sidebar → nobody
Priority: P3 → --
QA Contact: sidebar
Target Milestone: Future → ---

Comment 42

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Status: NEW → UNCONFIRMED

Comment 43

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 44

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 45

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 46

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 47

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 48

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 49

9 years ago
MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago.

Because of this, we're resolving the bug as EXPIRED.

If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component.

Query tag for this change: EXPIRED-20100420
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → EXPIRED
Still a serious issue!
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: EXPIRED → ---

Updated

8 years ago
Status: REOPENED → NEW
You need to log in before you can comment on or make changes to this bug.