Open Bug 48583 Opened 24 years ago Updated 14 years ago

sidebarURLSecurityCheck should use nsIScriptSecurityManager.idl

Categories

(SeaMonkey :: Sidebar, defect)

Product:
SeaMonkey
Component:
Sidebar
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

People

(Reporter: nicolas.chauveau, Unassigned)

References

(
URL
)

Details

(Keywords: arch)

Attachments

(1 obsolete file) 

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
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
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
Marking as NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
can you try this again and see if it works, not sure if this is an issue or not.
Assignee: kerz → matt
Keywords: nsbeta1
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.

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.
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. ***
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...

*** 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.
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
URL: http://bonsai.mozilla.org/cvsblame.cg...
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
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.

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).
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]
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.

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?
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.
*** Bug 92703 has been marked as a duplicate of this bug. ***
Attached patch proposed patch (obsolete) — — Splinter Review 

    
    

    
  
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?

    
    

    
  
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

    
    

    
  
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. ;-)



    
    

    
  
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?


    
    

    
  
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

    
    

    
  
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]

    
    

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

    
    

    
  
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

    
    

    
  
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)


    
  

        Attachment #43915 -
        Attachment is obsolete: true

    
    

    
  
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 → ---

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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

    
    

    
  
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
Closed: 14 years ago
Resolution: --- → EXPIRED

    
    

    
  
Still a serious issue!
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: EXPIRED → ---

    
  
Status: REOPENED → NEW

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: