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

Changes to @grant and @inject-into #265

Closed
quoid opened this issue Jun 2, 2022 · 49 comments
Closed

Changes to @grant and @inject-into #265

quoid opened this issue Jun 2, 2022 · 49 comments

Comments

@quoid
Copy link
Owner

quoid commented Jun 2, 2022

The next patch update will change a few things, that while programmatically minor (from a development standpoint), that can change the way your userscripts work. Since I don't have discussions enabled on this repo, this issue is more of a public announcement, but comments and discussion are more than welcome. Here are the changes:

  • content is the new default value for the @inject-into metadata key (i.e. if you don't include @inject-into the value, content, will be assumed)
  • when @grant has any value, @inject-into values of page and @inject-into values of auto will automatically be reverted to @inject-into content (a message will be printed to the browser console when this occurs)
  • users can still inject into the page context, however they can not use GM apis in the page context

The reason these changes are being implemented is to ensure the the page context can not gain access to the privileged GM apis.

Related issue discussion: #252

@quoid quoid added this to the macOS 4.2.1, iOS 1.2.1 milestone Jun 2, 2022
@quoid quoid pinned this issue Jun 2, 2022
@ACTCD
Copy link
Collaborator

ACTCD commented Jun 2, 2022

Just as a hint, if this change causes your script to not work, for most scripts that used to work fine in quoid/userscripts, you may just need to explicitly declare @inject-into page in the script after to resume work.

For some other cases, you may be using both GM APIs and page scripts, please try to separate your script logic, manually add page scripts to the page DOM, and keep the whole script in the content script context to keep everything working.

For the remaining cases, you can describe your use case and maybe we can tell you the right way to implement it. Always remember that unsafeWindow is unsafe, properly designed user scripts do not need and should not use it, and it may not be implemented correctly in Safari at this time, so it is currently not supported.

@KamilTheDev
Copy link

Please add an option to "enable unsafe mode", so that Userscripts will work as they did in the prior version.

After updating to v1.2.1, most of my userscripts are not working.

I would happily downgrade back to v1.2.0, but unfortunately iOS doesn't let you.

@quoid
Copy link
Owner Author

quoid commented Jul 20, 2022

@KamilTheDev

Can you give me an example of a userscript that worked prior but doesn't work any more? Preferably something simple?

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 20, 2022

@KamilTheDev

Please add an option to "enable unsafe mode"

I don't think it's a sensible option, you can never expect users to understand the risks behind this option, most users just want it to work by default and ignore the security implications until dire consequences.

Fix security bugs and steer everything towards the correct implementation. It must be a difficult process, but it's ultimately beneficial to the user. Maybe you should feedback those script authors to implement functionality in a safe way, which they can.

And as mentioned above, most scripts only need to declare the corresponding metadata, or make few modifications to run in a more secure and generic way, and a few scripts that rely on non-generic features need to redesign their logic.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 20, 2022

Just for the average user, I would like to tell you the nature of these changes more simply with an analogy.

These changes is not like the iOS system does not allow users to use root permissions for security or other reasons, it is more like closing a path that allows anyone to use root permissions to control you, which means it shouldn't exist at all. ('root' in this case means higher privileges)

These changes does not prohibit your rights, but protects you from malicious scripts. This was never an option.

@quoid
Copy link
Owner Author

quoid commented Jul 20, 2022

The main goal of this change was to isolate the GM apis from the page context and that's what was done. You don't want the page to be able to execute those methods.

If a developer wants the expose those methods to the page context, I can't stop them from doing so, but that functionality is no longer built into the extension to do so. A motivated developer can still expose those function to the page, but the effort required to do so might be better spent elsewhere.

If anyone has any suggestions on how to improve the functionality to securely give the page access to the methods (if that even exists) or how to pull the page context into the context of the content script, please do share.

It is very possible that a minor change in your existing userscript will restore functionality. I apologize for the inconvenience this has caused anyone.

@quoid
Copy link
Owner Author

quoid commented Jul 21, 2022

Another note:

I reverted the default @inject-into value to auto which tries to inject in the page scope first. Now that the GM APIs are properly isolated, I think should be OK and this matches ViolentMonkey's default behaviour

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 21, 2022

@quoid Yes, as long as @grant is not declared or its value is none, it should be OK. This should significantly improve compatibility for most scripts. As far as I can tell, this will also matches Tampermonkey's default behavior.

@KamilTheDev
Copy link

I reverted the default @inject-into value to auto which tries to inject in the page scope first.

Thanks, that was the cause of the problems. Also, @grant none causes @inject-into page to be disallowed, that needs to be fixed.

With these changes, will something like a Popup Blocker userscript be able to work? It overrides window.open, uses GM_*Value for storing values, appends HTML to page, etc.

Example userscript: https://github.com/Eskander/ultra-popup-blocker/blob/main/src/ultra-popup-blocker.user.js

Well, I'm not sure if that userscript would work either way, if synchronous GM functions aren't supported.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 21, 2022

@KamilTheDev I think with some proper modifications it should work, it just needs to separate out the logic of the page script in the content script context, add this part of the code as an in-line script to the page document as an alternative to unsafeWindow , it should just work. That is, it needs to run in @inject-into content. I also noticed that it might also require replacing the GM_ APIs to GM. APIs.

I see that @grant none problem has been fixed with #291.

@scholtzm
Copy link

scholtzm commented Aug 7, 2022

Is there a way to have access to GM API but also to page's context?

@ACTCD
Copy link
Collaborator

ACTCD commented Aug 7, 2022

@scholtzm The best way is to describe your use case in detail.

@scholtzm
Copy link

scholtzm commented Aug 8, 2022

@ACTCD I use GM_xmlhttpRequest to load cross-origin resources but I also use some globals used by the website available in the page context. This is possible in Tampermonkey and I wanted to provide the script for Userscripts users on iOS.

@ACTCD
Copy link
Collaborator

ACTCD commented Aug 9, 2022

@scholtzm Still, your use case is not clear enough, can you provide your script, or a more detailed description of what you're doing, so I might be able to help you separate out the logic.

To be clear, unsafeWindow is not possible in Safari for now, which is not supported in Userscripts, so you can't use the same method work in Userscripts like the extensions for other browsers, but unsafeWindow is not necessary in most cases, you just need to detach properly your script's logic, most of the time it will work fine in Userscripts.

@scholtzm
Copy link

scholtzm commented Aug 12, 2022

@quoid Perhaps there could be a way to preserve @inject-into page even if there are some grants for GM API?

This way I could have a single script for both Tampermonkey and Userscripts users. Right now, all @grants are honoured and @inject-into is not, but I would like to achieve the exact opposite -- honour @inject-into and disregard @grants.

@ACTCD
Copy link
Collaborator

ACTCD commented Aug 12, 2022

@scholtzm

Perhaps there could be a way to preserve @inject-into page even if there are some grants for GM API?

Other browser extensions do this via unsafeWindow, which as I said above is currently not possible in Safari. The previous implementation had security risks, which is why this change happened. If there is a safe way to implement unsafeWindow, contributions are welcome.

Anyway, most scripts don't need unsafeWindow at all to achieve what they do, if you specify the details of your function, we might be able to tell you how to do it without unsafeWindow, other than that, we can't provide more help.

This way I could have a single script for both Tampermonkey and Userscripts users. Right now, all @grants are honoured and @inject-into is not, but I would like to achieve the exact opposite -- honour @inject-into and disregard @grants.

Userscripts behaves in the same way as Tampermonkey, as I mentioned above, they are also not injected into the page after @grant the high-level API, restricted to the context of the content script. Disregard @grant The most straightforward way is to delete it in the metadata, it's easy.

@scholtzm
Copy link

scholtzm commented Aug 12, 2022

Userscripts behaves in the same way as Tampermonkey.

It certainly does not otherwise the same script that works in Tampermonkey would also work in Userscripts.

Disregard @grant The most straightforward way is to delete it in the metadata, it's easy.

Then I would need to distribute 2 different scripts.

I made a mistake of tagging you in my previous post, fixed it.

@quoid

This comment was marked as outdated.

@scholtzm
Copy link

Thank you @quoid for a thorough explanation.

Your example game me another idea, how about calling page globals from userscript that is using @inject-into content via the script tag injection?

Example:
(The page I tested this on uses jQuery and has $ in global scope.)

// ==UserScript==
// @name        Test - call page from content scope
// @description This is your new file, start writing code
// @match       *://*/*
// @inject-into content
// ==/UserScript==

console.log('from content', typeof $);

const code = `
  console.log('from page', typeof $, $.toString());
`;

const tag = document.createElement("script");
tag.textContent = code;
document.head.appendChild(tag);
tag.remove();

image

Is this an OK approach to take? I might be missing some crucial detail why this is not a good idea or won't work properly.

@quoid
Copy link
Owner Author

quoid commented Aug 12, 2022

@scholtzm

Your example looks similar to what I did, I don't see any difference. You could update the page globals through a script tag (if needed) and it would be better than my example because you don't expose the GM api to the page scope. Although, my example was just trying to show how you could get a GM api into the page scope.

I might be missing some crucial detail why this is not a good idea or won't work properly.

You can still face an issue with the page CSP. If the page has a strict CSP, then whatever you inject through a script tag won't get executed. There are ways to avoid page CSP issues, one of which I believe TamperMonkey uses which is header manipulation, but as mentioned above that's not possible with Safari. Another way is by using web_accessible_resources but attempting to bake that into this extension would incredibly difficult if not, impossible.

At a higher level, the main reason we avoid GM apis in the page scope is because that could allow page javascript to gain access to the privileged APIs and to the background/content script scope of the extension. Any nefarious code running on a page could do a lot of bad things if it had that kind of access.

@scholtzm
Copy link

scholtzm commented Aug 12, 2022

@quoid I took a stab implementing it in the script but I'm not sure I can make it work. My initial idea was shortsighted and didn't account for needs in my script. Sometimes I need to retrieve value from page context and in one case I actually pass my own callback function as an argument.

Makes me wonder again about unsafePage. It would solve a lot of issues for scripts that require this functionality. I don't see much difference between using unsafePage and exposing GM API as shown in your example other than shifting the responsibility from script to meta. Though the final decision is up to you of course.

@quoid
Copy link
Owner Author

quoid commented Aug 12, 2022

@scholtzm

I am pushing a beta build that includes the above changes, if you're not on beta you can sign up from links on the bottom of the README or shoot me an email (on my profile page)

@scholtzm
Copy link

Yep, this is what I meant with my comments. Will check out the BETA build.

@scholtzm
Copy link

scholtzm commented Aug 14, 2022

Tested both macOS and iOS Betas and can confirm it works as expected for my use case. 👍 I'm looking forward to seeing this update in the stores for everyone else.

@quoid
Copy link
Owner Author

quoid commented Aug 15, 2022

@scholtzm after a week or so I will push to app store

@scholtzm
Copy link

@scholtzm after a week or so I will push to app store

Is there any chance the release will happen in the upcoming week?

@quoid
Copy link
Owner Author

quoid commented Aug 28, 2022

@scholtzm I can submit it to today. It usually takes a day or so to get pushed as an update to users.

@quoid
Copy link
Owner Author

quoid commented Aug 30, 2022

closing this again, the latest build contains the newest changes

@ACTCD
Copy link
Collaborator

ACTCD commented Sep 9, 2022

@quoid

After reading the commit related to this issue, and the source code related to adding GM APIs to user scripts, I see that you did not fully fix the issue that caused the security risk.

You still use window.postMessage in the content script context to pass the GM APIs call, then the page script only needs to listen to the Message event to obtain the current US_uid, and then easily call the GM APIs this should only be @grant in content scripts.

Since GM APIs are only allowed in content scripts, you should stop using window.postMessage to pass calls, you should directly use browser.runtime.sendMessage to pass calls, and handle the return results.

@quoid
Copy link
Owner Author

quoid commented Sep 9, 2022

@ACTCD

You still use window.postMessage in the content script context to pass the GM APIs call, then the page script only needs to listen to the Message event to obtain the current US_uid, and then easily call the GM APIs this should only be @grant in content scripts.

The content script scope is isolated from the page scope, they have different window objects. Are you saying that even thought the window object is a completely different in the content script scope, somehow the page scope can still listen to message events there.

Since GM APIs are only allowed in content scripts, you should stop using window.postMessage to pass calls, you should directly use browser.runtime.sendMessage to pass calls, and handle the return results.

Yes, that should still be done, even if not a security concern.

@ACTCD
Copy link
Collaborator

ACTCD commented Sep 9, 2022

@quoid

Userscripts currently supports the following api methods. All methods are asynchronous unless otherwise noted. **All methods are accessible without regard to `@grant` when `@inject-into` has the `content` value.**.

From what I've read in the relevant source code, there seems to be a description error here, @grant is still required in the context of content scripts, not all methods can be accessed directly without regard to @grant.

According to the description here, @grant should also be required in the context of content scripts.

@ACTCD
Copy link
Collaborator

ACTCD commented Sep 9, 2022

@quoid

The content script scope is isolated from the page scope, they have different window objects. Are you saying that even thought the window object is a completely different in the content script scope, somehow the page scope can still listen to message events there.

Yes, I wrote a simple script to test, the page script can completely listen to the message sent in the content script.

The instructions here also warn ⚠️ this situation:

It is not possible for content or web context scripts to specify a targetOrigin to communicate directly with an extension (either the background script or a content script). Web or content scripts can use window.postMessage with a targetOrigin of "*" to broadcast to every listener, but this is discouraged, since an extension cannot be certain the origin of such messages, and other listeners (including those you do not control) can listen in.

Content scripts should use runtime.sendMessage to communicate with the background script. Web context scripts can use custom events to communicate with content scripts (with randomly generated event names, if needed, to prevent snooping from the guest page).

@quoid
Copy link
Owner Author

quoid commented Sep 9, 2022

@ACTCD

Yes, I wrote a simple script to test, the page script can completely listen to the message sent in the content script.

Could you send me that script? My email is on my profile page or you could post it here

EDIT:

I wrote a test script as well and was able to log the messages being posted from the content script scope. However, I was not able to execute privileged APIs from the page scope, which was my main concern.

This will be corrected in an update soon.

@quoid
Copy link
Owner Author

quoid commented Sep 9, 2022

@ACTCD

The update readme text will be:

Userscripts currently supports the following api methods. All methods are asynchronous unless otherwise noted. Users must @grant these methods in order to use them in a userscript. When using API methods, it's only possible to inject into the content script scope due to security concerns.

quoid added a commit that referenced this issue Sep 9, 2022
@ACTCD
Copy link
Collaborator

ACTCD commented Sep 9, 2022

@quoid

However, I was not able to execute privileged APIs from the page scope, which was my main concern.

As long as any script is injected into the current page, even if it's just @inject-into content, and it @grant some APIs, the page script can get the US_uid by listening to the message whenever it is called, and then it can directly constructs the same window.postMessage message to call privileged APIs.

@quoid
Copy link
Owner Author

quoid commented Sep 9, 2022

the page script can get the US_uid by listening to the message whenever it is called, and then it can directly constructs the same window.postMessage message to call privileged APIs.

I should have elaborated to say, arbitrarily execute API methods. What you mention above, although unlikely, is a possibility and #315 should resolve it.

@ACTCD

This comment was marked as resolved.

@quoid
Copy link
Owner Author

quoid commented Sep 9, 2022

@ACTCD

I understand what you are saying and agree. This will get fixed. Feel free to email me whenever with sensitive information.

@MotherOfClamperl

This comment was marked as outdated.

@quoid
Copy link
Owner Author

quoid commented Jul 21, 2023

@MotherOfClamperl that's a fairly old snippet and I don't believe it should work in more recent versions.

@MotherOfClamperl
Copy link

@quoid yeah only just tested it on a "page" script and got this error: TypeError: undefined is not an object (evaluating 'browser.runtime.onConnect.addListener')

...so there's no ways left to expose xmlhttpRequest at the very least?

I'd like to have userscripts that have persistent data/settings without needing any additional content scripts in addition to the page script.

I could just use http://localhost + fetch rest API, but the userscripts are synced to my phone, and it would suck having 0 settings in some userscripts without additional content scripts. Only way around this I could think of is running an additional server on iOS using localhost (since connecting via network (eg: http://192.168.x.x) triggers CORS with fetch, as well). But I don't think that's possible...?

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 21, 2023

@MotherOfClamperl I'm not sure about your specific use case, but we don't recommend that you expose any of the GM Advanced APIs to the page script context.

Refer to this demo if you need to exchange data in page script context and content script context.

@umangkumarr
Copy link

Screenshot 2023-07-24 at 09 14 59 Screenshot 2023-07-24 at 09 16 01

If someone could help me with this.

Link to userscript: https://greasyfork.org/scripts/38050-cf-predictor/code/CF-Predictor.user.js

System Information:

macOS : Version 13.4.1 (c)
Userscripts Safari Version 4.4.2 (73)

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 24, 2023

@umangkumarr

It looks like your script is using jQuery, but that doesn't exist in the content script context. Try import jQuery lib using @require metadata, this should help you resolve the issue.

@ACTCD ACTCD unpinned this issue Nov 30, 2023
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

6 participants