423890 - Message 'Security Manager vetoed action' does not have enough information

Status: RESOLVED INCOMPLETE

(Core :: XPConnect, defect)

Description

[John J. Barton]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030317 Firefox/3.0b4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030317 Firefox/3.0b4

With FF3.0b5pre and Firebug 1.1.0b12 from http://getfirebug.com/releases
the Firebug command line fails for simple expressions like "var a;". The message is new and I don't know what to do with it:

commandLine.evaluate FAILS: [Exception... "Security Manager vetoed action"  nsresult: "0x80570027 (NS_ERROR_XPC_SECURITY_MANAGER_VETO)" location: "JS frame :: chrome://firebug/content/commandLine.js :: <TOP_LEVEL> :: line 100"  data: no]
with (__win__.__scope__.vars) { with (__win__.__scope__.api) { with (__win__.__scope__.userVars) { with (__win__) {try {__win__.__scope__.callback(eval(__win__.__scope__.expr));} catch (exc) {__win__.__scope__.callback(exc, true);}}}}}
Line 0

Line 100 for commandLine.js is

    Components.utils.evalInSandbox(scriptToEval, sandbox);

I've tried a several variations but so far I can't narrow it down. If I understood what the error message was trying to say it would help. Any hints? (The code works in FF3b4 and FF2).

Reproducible: Always

Steps to Reproduce:
1.Install http://www.getfirebug.com/releases/firebug/1.1/firebug-1.1.0b12.xpi
2.Go to any web page, enable firebug (lower right corner check mark icon)
3.Select "Console" panel
4. At the arrow box >>> type "var a;" enter-key
Actual Results:  
Exception as above.  But the error message does not have enough information for the developer to fix the problem.

Maybe related to 307984 since its possible that a better error message from evalInSandbox would allow me to find the problem.

Comment 1

[John J. Barton]

I tried to make a smaller test case before filing this report but I was unsuccessful.  I need more info to decide what might be important to trigger the message.

Comment 2

[Michael Kurze [:michaelk]]

Hi, maybe this helps to track down the problem:

Using firebug 1.1.0b12 and Mac OS 10.5, this is WFM prior to and including:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008030404 Minefield/3.0b5pre

I start experiencing the problem with march 5th builds (and newer):
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008030504 Minefield/3.0b5pre

FWIW, there is another change in command line evaluation that is possibly related to the underlying error which is hidden by the Exception. Trying to use the firebug command line in a chrome context such as (chrome://firebug/content/firebug.xul) yields an "EvalError: function eval must be called directly, and not by way of a function of another name", also starting with march 5th builds.

This does of course not affect the error message being insufficient.

Comment 3

[Mike Shaver (:shaver emeritus)]

mrbkap helped me figure this out, because he's awesome.

Firebug-as-chrome sets win.__scope__ to contain some privileged (?) helper functions, and then runs evalInSandbox("with (win.__scope__) cmdlinescript").  win is a SJOW (since March 5th's builds; bug 344494), and so __scope__ is set to an object that requires chrome privileges to access.  When the script is evaluated with content privileges, it doesn't have those privileges, which keeps it (or the web page, happily) from being able to chase __scope__'s links out to things it shouldn't be able to access.  The "keeps it" manifests as the VETO error.

Some options:
1) lose the win.__scope__ stuff (and utility functions), and just call evalInSandbox to evaluate the script in the context of the web page.  Simplicity may be a virtue!

2) create a sandbox around the window, and then call sandbox.importFunction on all of the functions you want to make available.  After you call eIS, wrap the sandbox in a SJOW if you want to peek at it more, and wrap the eIS return in a SJOW. (See nsProxyAutoConfig.js via mxr for examples of this pattern; call the constructor without "new" if you want to let primitives pass through as the return value.)  SJOW-wrapping of eIS return will hopefully be automatic before we ship FF3, but doing it manually here will work before and after that.

John: we don't quite understand this comment:

// XXXjjb FF3  needs win.__scope__ because we eval in sandbox

What about using evalInSandbox necessitates win.__scope__?  And why eval inside the string that you pass to evalInSandbox?  Does that solve a problem I haven't thought of yet?

Comment 4

[John J. Barton]

I am going to try to eliminate evalInSandbox. I don't understand it and it does not reproduce the environment of the web page even with the __scope__ additions.

But back to the bug at hand, the problem illustrated by this example is clear: it took the two experts in Mozilla to figure out what the error message meant. 

Comment 5

[Mike Shaver (:shaver emeritus)]

What do you mean by "does not reproduce the environment of the web page"?  What are the differences?  You should absolutely keep using eIS if you're going to run code in the context of a web page.  win.__scope__ isn't part of the environment of the web page, which is likely why Firebug explicitly injects it.  If you want the environment of the web page, why not just evalInSandbox(cmdlinestring, sandbox) without all the win.__scope__ injection and with complexity?

(It wasn't that hard to figure out what the error meant -- "content can't access __scope__" -- though we should provide a better error message for sure.  The hard part was figuring out what Firebug was trying to do, and why win.__scope__ is made necessary by eIS, which I still don't understand!)

Comment 6

[John J. Barton]

I'd have to go back and take out the __scope__ to remember all of the issues with evalInSandbox.  Here are some of the pending ones:
http://code.google.com/p/fbug/issues/detail?id=521&q=label:commandline
http://code.google.com/p/fbug/issues/detail?id=513&q=label:commandline
http://code.google.com/p/fbug/issues/detail?id=459&q=label:commandline
http://code.google.com/p/fbug/issues/detail?id=428&q=label:commandline
http://code.google.com/p/fbug/issues/detail?id=379&q=label:commandline

But for me the real issue with evalInSandbox is simple: I can't predict what it will do.  Perhaps it is as simple as you say.

If the message had been "content can't access __scope__" I would not have submitted this bug report. But that wasn't the message. So how can you say 
"It wasn't that hard to figure out what the error meant" when it took you and mrbkap to get this far?

Or a possible resolution here is duplicate of Bug 307984, if you think that this error message would point to the error if that bug was fixed.

Comment 7

[Mike Shaver (:shaver emeritus)]

I'm saying that it didn't take two experts to figure out that much (the stack trace clearly showed that we were failing on CanAccess to __scope__) -- I needed mrbkap to help me unwind what what being set up by Firebug.  Anyway, yes, we definitely need a better error message, distinct from 307984's line number woes.

What predictions do you want to make about eIS?  Can you give an example of it behaving differently from, f.e., a <script> element inserted in the page?  What part of the web page's environment -- of which we agree that __scope__ isn't a part, right? -- isn't being reflected?  I'd like to have good docs on eIS, but I'm not clear on what's not clear.

Comment 8

[John J. Barton]

(In reply to comment #7)
> I'm saying that it didn't take two experts to figure out that much (the stack
> trace clearly showed that we were failing on CanAccess to __scope__) -- I

Which stack trace? What is "CanAccess"?

Comment 9

[John J. Barton]

(In reply to comment #7)
> What predictions do you want to make about eIS?  Can you give an example of it
> behaving differently from, f.e., a <script> element inserted in the page?  What
> part of the web page's environment -- of which we agree that __scope__ isn't a
> part, right? -- isn't being reflected?  I'd like to have good docs on eIS, but
> I'm not clear on what's not clear.
> 
According to http://developer.mozilla.org/en/docs/Components.utils.evalInSandbox,

"The sandbox is just an empty JavaScript object marked as being created by the restricted privilege principal. It will become the global scope object when you pass it to evalInSandbox(text, sandbox)." 

Therefore it would seem to me that if you want the text to be evaluated in the scope of the web page, then you need to put 'window' into the sandbox. Is that in correct?

A bit further down in the same document:
"On the other hand, any function that comes out of the sandbox executes with the privileges of chrome code. This can happen in a surprising number of ways as you can see in the next section. "

Which I interpret thus: your problems in getting evalInSandbox to work for all of the users is a waste of time because the command line will be insecure "in a surprising number of ways".  If the document said "4 ways" at least I would feel a bit more convinced.  

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #9)
> Therefore it would seem to me that if you want the text to be evaluated in the
> scope of the web page, then you need to put 'window' into the sandbox. Is that
> in correct?

Yes. The idea is that you create the sandbox object, which starts out empty, and then provide the API you want to your consumer. For example, GreaseMonkey, which uses evalInSandbox to evaluate user scripts sets sandbox.__proto__ = window to provide access to the window APIs (note that in GM's case, window is an XPCNativeWrapper, which might not be what you want).

> Which I interpret thus: your problems in getting evalInSandbox to work for all
> of the users is a waste of time because the command line will be insecure "in a
> surprising number of ways".  If the document said "4 ways" at least I would
> feel a bit more convinced.  

I don't understand your sentiment here. You seem to want us to be able to provide a 100% guarantee that "if you use this API, you will not have security problems," but that's simply not possible. The reason that the document doesn't say "4 ways" is because JavaScript actually provides an extremely large attack surface (mostly thanks to getters, setters, and the fact that most functions and objects can be overwritten). We provide wrappers and functions like evalInSandbox to mitigate the problem, but that doesn't absolve you, as an extension developer, from thinking about security.

Further, you seem to be of the mind that "if evalInSandbox doesn't magically fix all of my problems, then it's not worth using it at all." evalInSandbox does provide additional security when used correctly and bug 386635 will make it easier to use. Also, as we improve the API, your extension will benefit, as opposed to some home-brew solution which you're on the hook for.

Comment 11

[John J. Barton]

I guess I lead this bug report off track. The title is still correct but most of what we said here does not relate to the problem.

Just to restate: the problem with the message is that I don't know what to fix when I get the message.  At the least I need to know what principal is in control and what principal is attached to the failed function.

It does not block progress on firebug, but it will be a shame if we have to live with it.

Comment 12

[Brian Crowder]

I'll try and bang out something better here, but this doesn't and shouldn't block, and might have to wait for a dot-release.

Comment 14

[John J. Barton]

I hit this problem again today. evalInSandbox fails with "Security Manager vetoed action".  Why?

Comment 15

[Brian Crowder]

John:  What were you trying to evalInSandbox?

Comment 16

[John J. Barton]

 From 
https://fbug.googlecode.com/svn/branches/firebug1.2/content/firebug/console.js
            var script = "";
            if (ff3)
            {
                script += "window.__defineGetter__('console', function() {\n";
                script += " return window.loadFirebugConsole(); })\n\n";
            }

            script += "window.loadFirebugConsole = function() {\n";
            script += " if (window._FirebugConsole) return window._firebug;\n";
            script += " var event = document.createEvent('Events');\n";
            script += " event.initEvent('loadFirebugConsole', true, false);\n"
            script += " window.dispatchEvent(event);\n";

            // If not ff3 initialize "console" property.
            if (!ff3)
                script += " window.console = window._firebug;\n";

            script += " return window._firebug };\n";

Comment 17

[John J. Barton]

(In reply to comment #15)
> John:  What were you trying to evalInSandbox?
> 
But just so we don't go off track, the bug here is not about evalInSandbox, its about the error message which does not have enough information for developers to take action.

Comment 18

[Brian Crowder]

If you catch the exception and dump it's stack property, what results do you see?

Comment 19

[John J. Barton]

(In reply to comment #18)
> If you catch the exception and dump it's stack property, what results do you
> see?
> 
It does not have a stack property:
commandLine.evaluate FAILED: sees object with typeof: 'object'; object contains:
[message]=Security Manager vetoed action;
[result]=2153185319;
[name]=NS_ERROR_XPC_SECURITY_MANAGER_VETO;
[filename]=chrome://firebug/content/commandLine.js;
[lineNumber]=152;
[columnNumber]=0;
[location]=JS frame :: chrome://firebug/content/commandLine.js :: <TOP_LEVEL> :: line 152;
[inner]=null;
[data]=null;
[initialize]=function initialize() {
    [native code]
};

Comment 20

[Brian Crowder]

John J. Barton:  How's the story here, now?  Does this error message still need improvement?

Comment 21

[John J. Barton]

Brian, Bug 484459 pushed me over the edge: I removed our code that uses evalInSandBox. So we won't see the error messages and I can't say how they look now.  You can close this report as far as I am concerned.

Comment 22

[John J. Barton]

I guess the resolution should be wontfix, there is no patch and no test that it works now.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

WONTFIX means "we wouldn't take a patch" - that doesn't apply here.