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

Luminous injects scripts into non-html files. #107

Closed
StaticallyTypedRice opened this issue Mar 19, 2019 · 9 comments
Closed

Luminous injects scripts into non-html files. #107

StaticallyTypedRice opened this issue Mar 19, 2019 · 9 comments

Comments

@StaticallyTypedRice
Copy link

Currently in Firefox (and I assume other browsers), Luminous injects its blocking code into all files loaded from the URL bar, including XML, JSON, text and image files. This causes these files to be corrupted as the injection messes with the syntax or data structure.

You can see it most clearly when you visit a text file with Luminous enabled: https://github.com/robots.txt

@ghostwords
Copy link

Fixed in Privacy Badger by EFForg/privacybadger#1954

@StaticallyTypedRice
Copy link
Author

Fixed in Privacy Badger by EFForg/privacybadger#1954

Seems straight forward enough. When I have time I'll try integrating the script into the injection controller and sending a pull request. There seems to be a boolean controlling whether whether luminous injects its scripts. I'll try to find where the rest if the injection conditions are and put this code there.

@gbaptista
Copy link
Owner

@ghostwords Thank you for sharing your solution!

Hi @StaticallyTypedRice! Thank you for reporting the issue and for creating a pull request with a possible solution!

In the case of luminous the issue is related to the support for the filterResponseData API (filterResponseData support #86).

The code responsible for injecting scripts into the wrong places is this:

  • js/background/injections/content/filter_response_data.js

From my tests, we do not have that kind of problem with the strategy used in Chrome or in older versions of Firefox (the "cookie strategy").

You can get a better context on this here: Not being able to guarantee interception #55

The options I believe we have to solve this:

1 - Remove filterResponseData support
Personally, I do not like this option since without this support we diminish the guarantee that the codes will be intercepted. This is simple, just disable this injection strategy by setting injection_strategy = 'cookie'; in these files:

  • js/background/strategy.js
  • js/content/strategy.js

2 - Create an advanced option to disable this type of injection.
I started working on this idea here: [WIP] Advanced Options #109
filterResponseData is a relatively new API supported only on newer versions of Firefox. We can say that its use is experimental, so it would make sense to leave this as an option to be enabled only by those who really want to test this...

3 - Make filterResponseData work better.
This is the option that I believe to be the best. We have an injection with greater guarantees of operation and do not affect the opening of links that should not have injected codes.

Not sure how to do this yet... I opened a pull request indicating the points in the code that would need to be improved: [WIP] Improve filterResponseData #110

Thoughts?

@StaticallyTypedRice
Copy link
Author

Interesting. Since this is a privacy and security focused extension, it's better to have false positives than negatives. I'll try a few solutions when I have time, but if it's causes the extension to not be guaranteed to inject into places where the code is needed, it's a huge problem.

@gbaptista
Copy link
Owner

Ok, new idea.

Can we rely on the content_type of the request to decide if the code should be injected?

Examples:

  • text/html > inject
  • application/xhtml+xml > inject
  • text/plain > don't inject
  • image/svg+xml > don't inject

I'm doing an experiment, it seems promising so far: https://github.com/gbaptista/luminous/pull/110/files

@gbaptista
Copy link
Owner

Actually, I do need the solution shared by @ghostwords.

It does not happen in Firefox this way because I use the filterResponseData. But - not sure why - the Chrome requests for xml documents are injecting the code...

Fixed for Chrome: https://github.com/gbaptista/luminous/pull/110/files#diff-73fb9f3ddac1b537a0a71b3895bccfc7

Thank you again!

@gbaptista
Copy link
Owner

@StaticallyTypedRice I believe that in the last correction I made the problem was solved. Can you please check if the problem still happens on version 0.0.27? (already available on the stores for Firefox, Chrome, and Opera)

@StaticallyTypedRice
Copy link
Author

StaticallyTypedRice commented Apr 3, 2019

It seems to be fixed on Linux but I'm still seeing it on Windows.

Edit: Never mind, it's fixed everywhere!

@gbaptista
Copy link
Owner

awesome, thanks!

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

3 participants