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

RFE: a shared object among user-scripts #1278

Closed
teramako opened this issue Feb 3, 2011 · 11 comments
Closed

RFE: a shared object among user-scripts #1278

teramako opened this issue Feb 3, 2011 · 11 comments
Milestone

Comments

@teramako
Copy link

teramako commented Feb 3, 2011

Some user-scripts refer to some object defined by the other user-script.
e.g.)

like following: LDRize use functions defined by Minibuffer.

// Minibuffer
window.Minibuffer = {
  // ...
  addShortcutkey: function () {...}
  // ...
};


// LDRize
window.Minibuffer.addShortcutkey(...);

Since SHA: 6a4ffd5 commited, these cannot work. Because window object had been not same object on Firefox 3.6 and an object defined by the other sandbox cannot be refer on Firefox 4.0b.

Of course, We can use unsafeWindow, but it has security risk.
and I considered about using DataContainerEvent of DOM Event, but it also has security risk.

So I concluded that we need a certain object for sharing among only user-scripts.

here is a patch:

diff --git a/components/greasemonkey.js b/components/greasemonkey.js
index 0deb036..0307104 100644
--- a/components/greasemonkey.js
+++ b/components/greasemonkey.js
@@ -255,6 +255,7 @@ GM_GreasemonkeyService.prototype = {
     var xmlhttpRequester;
     var resources;
     var unsafeContentWin = wrappedContentWin.wrappedJSObject;
+    var sharedObject = {};

     // detect and grab reference to firebug console and context, if it exists
     var firebugConsole = this.getFirebugConsole(unsafeContentWin, chromeWin);
@@ -273,6 +274,7 @@ GM_GreasemonkeyService.prototype = {
       resources = new GM_Resources(script);

       sandbox.unsafeWindow = unsafeContentWin;
+      sandbox.sharedObject = sharedObject;

       // hack XPathResult since that is so commonly used
       sandbox.XPathResult = Ci.nsIDOMXPathResult;
@arantius
Copy link
Collaborator

arantius commented Feb 3, 2011

Can you be more specific about what you mean by "Since SHA: 6a4ffd5 commited"?

Can you provide a reduced test case demonstrating that this approach worked in some released version of Greasemonkey?

@teramako
Copy link
Author

teramako commented Feb 3, 2011

Sorry, I missed creating link.
SHA: 6a4ffd5
6a4ffd5
Each of sandbox's prototype are re-wrapped by this commit.

Test scripts are here https://gist.github.com/809658

until version 0.9, can get window.FOO value which is defined test01.user.js from test02.user.js.
but since the commit, cannot get value, returns undefined

@Martii
Copy link
Contributor

Martii commented Feb 6, 2011

See also:
#1258

@carellano001
Copy link

Teramako, I see and share your point of view.

In addition to that, I think that if the sharing of the window is maintained in the same fashion that exist before, there is no risk for scripts. If the scripts want to communicate one each other, they will use window properties. If the scripts don't want to communicate one each other, they don't have to do anything special...

@Martii
Copy link
Contributor

Martii commented Feb 7, 2011

If the chance exists, in any fashion, that is pointed out in #1258 of any kind of exploit on the GM API or an Authors code then I vote a -1 to restore the original method that was removed in GM 0.9.x. If there is the possibility utilizing some metadata key, say the @namespace (since it does not have to be unique by itself but in combination with @name), then I would be +1 for something along these lines provisionally.

@carellano001
Copy link

Also, it could be possible to support these communication using GM_get/setValue. GM_setValue can have another parameter that denotes if the stored data must be shared or not.

Example:

GM_setValue("sharedVar","value",true);
GM_setValue("NOsharedVar","value",false); 
GM_setValue("NOsharedVar","value"); //Backward comp.

I am just giving ideas to support desirable communication between scripts.

@sizzlemctwizzle
Copy link
Contributor

I've designed an example to illustrate the possible dangers of allowing scripts to share objects via expando properties on the window object. Use GM 0.9.0 or earlier(I used 0.8.6) to test these scripts. The order in which the scripts are executed makes no difference in my example.

My first script is merely a non-malicious script that carelessly assigns two functions as properties of the window object. This example is not unreasonable, since many users are likely unaware that doing so will share these functions with all scripts that are installed.

My second script is an example of a malicious script that is designed to target my first script. A real attacker is unlikely to make his intentions so obvious and could use various means to obfuscate the code. The code doesn't even have to be included in the script. It could be in an @require or loaded via xhr at run-time and evaled. The malicious script uses the functions of the first script in order to get itself access to the global of that script. Once it has done this it has access to all the GM api functions of that script. It could corrupt all the functions and make the script unusable. It could listen in on GM_xhr request, logging the information for itself and submitting to a remote server. It could even get any information the script might have stored in about:cache.

In my opinion, letting scripts share objects via expando properties on the window object is completely unsafe because all scripts will have access to these objects. Many users many not be aware that these properties are shared between scripts and may therefore carelessly add functions that can be exploited, which I have shown is entirely possible.

@arantius
Copy link
Collaborator

So:

A) *etValue is not a communication mechanism. If such a mechanism is built, it should probably not be layered there.
B) This sort of behavior (sharing objects via expando properties of window) was undocumented and unintended. Breaking scripts that relied on it is very undesirable.
C) But finally, denial of service and stability problems pretty much always trump everything else.

As described, I'm seeing that with this old unintended behavior, it was theoretically possible for one user script to DoS another (poorly written) script, including by altering its persistent *etValue store.

I do not, however, see any GM_xhr related problems. Assuming you are an installed user script, you can run all the GM_xhrs you want, and do whatever you like with the responses -- no need to snoop on existing calls.

@arantius
Copy link
Collaborator

This issue has been quiet for a long time. Is anyone out there still looking for this?

@Martii
Copy link
Contributor

Martii commented Jun 25, 2011

I am not sure myself. Most of the user.js ScriptWrights are doing their best to work together on compatibility fixes and shared libraries via @require. It would be nice to see something, perhaps a messaging pump built but as you stated it's pretty quiet.

@carellano001
Copy link

The undo of re-wrapping and shared window object only works in FF3. No longer works on FF7.

So the behaviour of some scripts changes depending on the FF version.

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

5 participants