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

SecurityError in barclaycardus.com #461

Closed
CrendKing opened this issue Aug 6, 2018 · 22 comments
Closed

SecurityError in barclaycardus.com #461

CrendKing opened this issue Aug 6, 2018 · 22 comments
Labels

Comments

@CrendKing
Copy link

CrendKing commented Aug 6, 2018

  • Browser:
    Firefox Nightly 2018-08-05
  • Operating System:
    Windows 10 64-bit
  • Screenshot:
    barclaycardus

Steps to reproduce:

  1. Create a style that applies to barclaycardus.com. The content does not matter. It could even be left empty. The bug is triggered when a style is injected.
  2. Visit https://cards.barclaycardus.com/
  3. The sign in window becomes empty. In console, I can see
SecurityError: The operation is insecure.
window.Modernizr</K</<        https://gif.barclaycardus.com/servicing/525d314b/js/base/bcusMod/build/globalDependencies.js:8:3504
y                             https://gif.barclaycardus.com/servicing/525d314b/js/base/bcusMod/build/globalDependencies.js:8:2159
window.Modernizr</K<          https://gif.barclaycardus.com/servicing/525d314b/js/base/bcusMod/build/globalDependencies.js:8:3442
window.Modernizr<             https://gif.barclaycardus.com/servicing/525d314b/js/base/bcusMod/build/globalDependencies.js:8:3400
<anonymous>                   https://gif.barclaycardus.com/servicing/525d314b/js/base/bcusMod/build/globalDependencies.js:8:19

and

Error: Script error for: handlebarsCore
http://requirejs.org/docs/errors.html#scripterror
C                    https://gif.barclaycardus.com/servicing/525d314b/js/base/vendor/requirejs/require.js:8:252
onScriptError        https://gif.barclaycardus.com/servicing/525d314b/js/base/vendor/requirejs/require.js:29:514
  1. If I disable the style, and leaving Stylus still enabled in addon, the problem is gone.

Version

It appears this bug only exhibits in Firefox Stylus 1.4.13. In Chrome Stylus 1.4.17 there is no problem.

Other information

  • Firefox xStyle 3.1.2 does not have this problem.
  • Once the page is loaded with the style disabled, enabling the style will both correctly apply the style, and not trigger any security error.
@stonecrusher
Copy link
Contributor

Can reproduce in FF 61.0.1 / Stylus 1.4.13.

@tophf
Copy link
Member

tophf commented Aug 6, 2018

All those errors are for the page scripts, not for our scripts. Stylus doesn't touch page scripts, doesn't use Modernizr, doesn't add/modify any page variables. Stylus only adds the style elements in the page DOM.

Personally I'm tired of FF-only bugs/quirks so I won't be investigating this.

@CrendKing
Copy link
Author

Exactly. That's why I'm very surprised when I isolated Stylus even with a Firefox 61 clean profile.

BTW, the sign in window is a frame with URL https://www.barclaycardus.com/servicing/authenticate/home?rnd=<random_number>. Some Firefox specific cross-domain security policy may contribute.

@tophf
Copy link
Member

tophf commented Aug 6, 2018

Er, will keep this one open instead.

@tophf tophf reopened this Aug 6, 2018
@tophf
Copy link
Member

tophf commented Aug 6, 2018

Turns out this site doesn't like there's a style element in the iframe.
xStyle is based on the old code which runs after a slight pause in iframes.
I'll try to add such a delay in iframes as it shouldn't introduce flickering.

@CrendKing
Copy link
Author

Thanks for the quick fix. Just curious, why is Chrome not affected?

@tophf
Copy link
Member

tophf commented Aug 6, 2018

I didn't investigate it. Maybe the site has different code for Chrome and there's a bug in the site code for FF. Or maybe there's a bug in FF that is triggered by the site code when it unexpectedly encounters our style element.

@CrendKing
Copy link
Author

CrendKing commented Aug 22, 2018

Mozilla finally approved the new version. I tried the 1.4.20 version from AMO. The homepage of Barclay US is indeed fixed. However, there are other pages in Barclay that still have the same "SecurityError", for instance https://www.barclaycardus.com/servicing/home?secureLogin= . Worse, when I login my account, the page is empty with "SecurityError".

The common factors of these two are

  1. There is no iframe in either page.
  2. Barclay uses Modernizr everywhere. Seems this issue is related to Barclay + Modernizr.

I'm not sure if Modernizr itself is incompatible with Stylus, or it is the Barclay's way of using Modernizr. The former should be relatively easy to check.

BTW, xStyle and Chrome Stylus work in both pages.

@tophf tophf reopened this Aug 22, 2018
@tophf tophf closed this as completed in d65b63a Aug 22, 2018
@tophf tophf reopened this Aug 22, 2018
@tophf
Copy link
Member

tophf commented Aug 22, 2018

I've tried a couple of things but none worked ultimately. It still looks like a bug in FF to me which is triggered by our style order protection, implemented only in Stylus.

Bisected to https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6c2e5d5828a5d7a089dd340f5a2cd250459a40f6&tochange=d811ce4ebcd9a7bfce3e6ac0ec5d006f9e2ff82c

Possible solutions:

  • report the bug to Modernizr and/or FF and hope they will be interested in investigating it (unlikely)
  • don't preserve style order on any Modernizr-enabled site (should be detectable via page-context global variable)
  • don't preserve style order on barclaycardus site (the easiest "solution")
  • add a global Stylus option to disable style order preservation, as an on/off switch or as a textbox with domains

@CrendKing
Copy link
Author

Option 3 looks like the easiest temporary fix. But Modernizr is relatively popular. Other websites might use it. Option 2 also looks like a clutch solution, as other Javascript libraries might do what Modernizr does. There is never an end.

I'm not familiar with this style order protection feature. Does it allow styles to be injected in specific order? If so, I do not see any reorder button in the extension to take advantage of it. In my opinion, if it is not a critical function, disabling it by default (for maximum compatibility) and provide option to enable it for whoever brave, basically option 4, seems the best way out. But you guys decide.

@Mottie
Copy link
Member

Mottie commented Aug 22, 2018

Option 5, add all the page styles into one big style tag. We might have to do this anyway if/when we switch to using browser.tabs.insertCSS.

@tophf
Copy link
Member

tophf commented Aug 22, 2018

Style order preservation ensures our styles are inserted in the order of their internal id and that they always follow <body> and other page styles, thus making our style win in same-specificity cases. This is quite important so I don't want to disable it by default. OTOH I don't care about FF or FF users so I don't mind disabling it in FF by default even though it's a bad decision.

switch to using browser.tabs.insertCSS.

In the future we might use contentScripts.register in FF or declartiveContent when it's implemented, both could be better than insertCSS.

@FeBe95
Copy link

FeBe95 commented Aug 22, 2018

Same issue seems to stop icloud.com/#reminders from loading properly. When I disable all styles for icloud or disable Stylus entirely the page loads just fine.
Error text: SecurityError: The operation is insecure.

OS: Windows x64
FF: 61.0.2 (64-Bit)

@tophf
Copy link
Member

tophf commented Aug 24, 2018

Traced the error on barclaycardus.com to an ancient buggy Modernizr build they're still using sourced from https://gist.github.com/srobbin/2773397

Firefox 59 (the bisect from above) decided to hide the contents of style elements added by an extension (thus violating DOM specification I believe but who am I anyway) and the buggy site script fails to access the last styleSheet element as it's now the one added by Stylus.

I didn't investigate icloud.com as I don't have an account, but it's likely they make the same assumptions about accessibility of styleSheets.

The current version of Modernizr wraps the styleSheet access code into try/catch so the majority of popular sites shouldn't be affected.

Back to possible solutions.

I don't like exposing such a low-level option in UI as only two sites were reported so far, one confirmed to be using an ancient buggy library (kinda their fault).

  1. We can disable style order preservation in Firefox on these two sites.
  2. We can enforce the order only when the page has loaded completely which may take a few seconds. It shouldn't affect a lot of styles or sites because most styles use !important anyway while most sites don't (at least not as often inside <body>).
    Here's a test version: zip, instructions. Please test, @FeBe95.

@FeBe95
Copy link

FeBe95 commented Aug 24, 2018

Tried the test version, the issue persists. (I disabled the current version before adding the test version temporally.)
Console output:

19:20:09.859 Reminders:  onerror exception: SecurityError:                                                       javascript-packed.js:281:1048 
             The operation is insecure. Script:
             https://www.icloud.com/applications/reminders/1815Project37/de-de/javascript-packed.js Line: 308
                                        
19:20:09.860 SecurityError: The operation is insecure.                                                           javascript-packed.js:308 

It's a minified script so this won't help much I think.
I have disabled the iCloud userstyle for now. Maybe Apple and/or Mozilla are going to fix it sometime without even knowing. If you have another idea on how to fix it I'd really appreciate it.

On more note about how icloud.com loads:
The main page https://www.icloud.com works as expected. Choosing "reminders" in the UI loads another view via ajax and also the affected javascript-packed.js. During this loading, an error is displayed.

I pasted the given error text to pastebin: https://pastebin.com/hi1dZuQp

grafik

@tophf
Copy link
Member

tophf commented Aug 24, 2018

Yep, that error indicates icloud fails due to the same reason while accessing document.styleSheets.
Try this one: zip

@FeBe95
Copy link

FeBe95 commented Aug 24, 2018

This workaround works as expected! Thanks!

@stonecrusher
Copy link
Contributor

Please add

// FRITZ!Box router logins
location.hostname === 'fritz.box' ||
location.hostname === '169.254.1.1' ||
location.hostname === '192.168.178.254' ||
location.hostname === '192.168.178.1' ||
location.hostname === '192.168.178.2' ||
location.hostname === '192.168.178.3' ||
location.hostname === '192.168.178.4' ||
location.hostname === '192.168.178.5' ||

to workaround the same problem on older routers like the FRITZ!Box 7170. Those routers by manufacturer AVM are very popular in germany with a market share of about 50%.

@tophf
Copy link
Member

tophf commented Sep 2, 2018

The numeric addresses are all from a private range so they can point to anything on a given computer, hence we can't hardcode it. I'll test the performance and maybe just enable the workaround by default in Firefox.

@DanaMW
Copy link

DanaMW commented Sep 2, 2018

Could they add that to there host file?

@CrendKing
Copy link
Author

@tophf Sorry for a late comment. Thanks for the fix. Since you have most knowledge here, could you open a bug to Mozilla to fix the behavior in Firefox? Ideally we should not need Firefox specific workaround in the code.

@eight04
Copy link
Collaborator

eight04 commented Oct 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants