Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eval() does not work as expected #1258

Closed
arantius opened this issue Jan 23, 2011 · 15 comments
Closed

eval() does not work as expected #1258

arantius opened this issue Jan 23, 2011 · 15 comments
Milestone

Comments

@arantius
Copy link
Collaborator

Originally: http://groups.google.com/group/greasemonkey-users/browse_thread/thread/3512884cac5c5eb2?hl=en#

On 2011-01-22 5:42 PM, Beho Double wrote:

Firefox 3.6.13 Greasemonkey 0.9.0

A simple script like this doesn't work .........

// ==UserScript==
// @name           test
// @namespace      http://test.free.fr
// @description    test
// @include        *
// ==/UserScript==

eval("alert('bop');");

....... but this script works with 2 alerts : :D

// ==UserScript==
// @name           test
// @namespace      http://test.free.fr
// @description    test
// @include        *
// ==/UserScript==

alert("bop");
eval("alert('bop');");
@arantius
Copy link
Collaborator Author

Install this script: https://gist.github.com/raw/792568/b5e41b5e66c1ba3e524c08c27d57bb07782f8249/eval%20test.user.js (Source: https://gist.github.com/792568)

I get alerts. In:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13

@arantius
Copy link
Collaborator Author

Ha, foolish of me, I missed double quotes around the alert. Edited (reflected at the link above) them in and confirmed:

Error: Security Manager vetoed action
Source File: file:///C:/.../gm_scripts/eval_test/eval_test.user.js
Line: 0

@sizzlemctwizzle
Copy link
Contributor

Confirmed that this error doesn't occur in greasemonkey/0.8 branch.

@Martii
Copy link
Contributor

Martii commented Jan 24, 2011

Single eval("alert('foo');"); does work in:

Mozilla/5.0 (X11; Linux x86_64; rv:2.0b10pre) Gecko/20110123 Firefox/4.0b10pre
"Dirty" profile (running profile but minimal add-ons and changes)
Greasemonkey 0.9.0 release

Single eval("alert('foo');"); does NOT work in:

Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
"Dirty" Profile (e.g. running profile)
Greasemonkey 0.9.0 release
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
"Clean" Profile (e.g. complete removal of .mozilla folder)
Greasemonkey 0.9.0 release
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
"Basic" Profile (e.g. basic security add-ons)
Greasemonkey 0.9.0 release

Error Console via right click, Copy context:

Error: Security Manager vetoed action
Source File: file:///~/.mozilla/firefox/{seed}.{profile}/gm_scripts/sandbox/sandbox.user.js
Line: 0

Line zero? It would appear that alert is only available within eval for 3.6.13 in the restricted scope (e.g. content scope using unsafeWindow)

I'm guessing in the history it happened around this commit forward.

Tested cacea0c ... it works and then the following commit at 807bea6 ... doesn't work.

@sizzlemctwizzle
Copy link
Contributor

Patch.

@sizzlemctwizzle
Copy link
Contributor

Workaround for a bug that causes the Security Manager to veto the use of eval. Closed by 6a4ffd5

@carellano001
Copy link

The change performed, crash other scripts. In version 0.90 and before, the window variable was shared between scripts. The following example works:
// ==UserScript==
// @name Script1
// @namespace http://test.free.fr
// @description Script1
// @include *
// ==/UserScript==

window.a=1;

// ==UserScript==
// @name           Script2
// @namespace      http://test.free.fr
// @description    Script2
// @include        *
// ==/UserScript==
alert(a);

And with this change, the second script crashes.

A workaround trying to solve the bug and maintaining the backward compatibility is:

var unsafeContentWin = wrappedContentWin.wrappedJSObject;
+    var safeContentWin = new XPCNativeWrapper(unsafeContentWin);

// detect and grab reference to firebug console and context, if it exists
var firebugConsole = this.getFirebugConsole(unsafeContentWin, chromeWin);

for (var i = 0; script = scripts[i]; i++) {
 ...
  // Re-wrap the window before assigning it to the sandbox.__proto__
  // This is a workaround for a bug in which the Security Manager 
  // vetoes the use of eval.
-     sandbox.__proto__ = new XPCNativeWrapper(unsafeContentWin);
 +    sandbox.__proto__ = safeContentWin;

@sizzlemctwizzle
Copy link
Contributor

And with this change, the second script crashes.

No, the second script crashes because your a variable in the second script is undefined. Variables are not shared between scripts, and we wouldn't want them to be.

The change you suggest does absolutely nothing anyway. You're just storing the re-wrapped window in a variable(safeContentWin) before assigning sandbox.proto to it. This wouldn't fix anything even if there were a bug, which there is not.

@carellano001
Copy link

I know the reason of the crash of the second script. I understand that the behaviour of share variables between scripts could be undesirable. But the previous example works before 0.91 and not with 0.91.

If the rewrapped window is reused (before 0.9), the properties asigned to the window variable by one script are available to the rest of the scripts. Also, from the scalability point of view, is better to reuse the rewrapped window than create one for each executed script...

I see your point, I want to know if you see my point.

@sizzlemctwizzle
Copy link
Contributor

But the previous example works before 0.91 and not with 0.91.

Wasn't aware of this, but this isn't the place to discuss this. Please create a topic in the gm dev group about this since only people who commented on this issue will ever be notified of your comments. We should get more opinions.

the properties asigned to the window variable by one script are available to the rest of the scripts.

Totally undesirable behavior imo. I'd rather that my scripts have a virgin namespace. Not to mention it requires scripts to be executed in a specific order to function properly. That'd cause all sorts of problems for users.

@Martii
Copy link
Contributor

Martii commented Feb 5, 2011

But the previous example works before 0.91 and not with 0.91.

I wasn't aware of this either. I would definitely like to read some other opinions and perhaps Aaron's take on this as well. Having this large of a "bullet hole" in the Sandbox/XPCNativeWrapper doesn't exactly appeal to me either.

@sizzlemctwizzle
Copy link
Contributor

Just for fun checkout this exploit by installing these two scripts: Script1 Script2

@Martii
Copy link
Contributor

Martii commented Feb 6, 2011

Hmmm I guess I did know vaguely about this according to the dev wiki page but I didn't realize about this "feature" of that methodology. Sharing window between the same @namespace wouldn't be all that bad and it is an idea that I brought up a few years ago on devjavu to be added to someone elses request for this. If you want to exploit your own code between Script A and Script B more power to you but if a 3rd party, non-same @namespace comes in and blows away the GM API or some other critical method it wouldn't be appealing at all.

See also:
#1278

@carellano001
Copy link

I used "window" variable to share information between scripts in a undocumented way but I think that is useful. For example, to load a common library for a bunch of scripts (I used in this way). I know that the order of execution matters but it could be resolved manually or with "@priority" attribute in the metadata block.

To sizzlemctwizzle: regarding to the example that you put before, in the second script, you don't need to put window.func(GM_getValue, GM_setValue) but func(GM_getValue, GM_setValue). If the "func" variable is not locally defined, the interpreter searches in "window" scope. The scripts can communicate if the want (for example, accessing window object), but no if they don't want (defining and using its local declaration).

To Martii: I agree with you. A mechanism to control the sharing must to be there, but this mechanism can't be implemented in the way that you say. The "@namespace" attribute must to be unique. It could be possible to be implemented with uri fragment identifier. For example if script1 has a @namespace http://www.test.com/#1 and script2 has a @namespace http://www.test.com/#2.

Thank you sizzlemctwizzle and Martii for your comments. I like GM :)

@arantius
Copy link
Collaborator Author

arantius commented Feb 7, 2011

This issue is closed, and is about a failure in eval().

Comments about the separate idea of "shared window between user scripts", should please please go in the issue where this was reported: #1278

dept42 pushed a commit to dept42/greasemonkey that referenced this issue May 12, 2011
arantius pushed a commit to arantius/greasemonkey that referenced this issue Aug 29, 2011
I've done my best to test everything mentioned, and eval() and expandos work as expected in Firefox 3-7.

Fixes greasemonkey#1278
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants