mlgualtieri / CSS-Exfil-Protection

Official repository for the CSS Exfil Protection browser extensions.
MIT License
155 stars 11 forks source link

Default styled form inputs in Firefox rendered incorrectly #13

Closed mlgualtieri closed 4 years ago

mlgualtieri commented 5 years ago

As per discussion in this Twitter thread (https://twitter.com/albertokernot/status/1116947194710560768) default styled form elements in Firefox are not properly styled until mousing over the elements.

mlgualtieri commented 5 years ago

A potential fix for this is in testing. The issue is caused by the CSS load blocker, which sets background-images to 'none' on input elements and relative input elements. Firefox adds a gradient to the button style that is impacted by setting this style to 'none'. It seems that there must be a bug in Firefox that does not re-apply the style to the button until the UI element is refreshed in some way.

A potential solution in testing is to set the background-image element to 'url()' instead of 'none'.

mlgualtieri commented 5 years ago

Actually, this is a bad way to fix the issue. By adding the empty url() parameter, it causes extra HTTP requests to the page for each element that this style is applied to.

mlgualtieri commented 5 years ago

I believe this issue may be 'fixed' by an update to Firefox. I'll do some more testing, but in my initial tests it appears to no longer be an issue.

mlgualtieri commented 4 years ago

Despite my previous comment, this remains an issue. A user provided the following example HTML. When loading the page the buttons remain unstyled. Hovering over buttons restores each to the default Firefox style, but sometimes strange artifacts like partially rendered buttons occur.

CSS_Exfil_test.html.txt

Thorin-Oakenpants commented 4 years ago

Mozilla have been in the process of switching over all the widgets from native themes

And with all that have lots of little styling, zooming, rendering teething bugs: so I guessing that's what you're encountering: the switch to non-native theming

This is a handy page Stephen put together, if it's any use to you

mlgualtieri commented 4 years ago

Thanks for the comment! I reviewed last week after you sent the links and never got a chance to reply. Yes the issue is that when an element isn't styled in Firefox, when removing all styles (required to ensure the CSS is sanitized before loading) it sometimes defaults to unstyled inputs instead of native styled inputs. I still don't have a great solution for this.

Thorin-Oakenpants commented 4 years ago

Yeah, I can see it now in http://stephenhorlander.com/form-controls.html when I toggle the extension on/off and reload - I'll see if I can ping someone from Mozilla

Update: FWIW, there's a widget.disable-native-theme-for-content pref which I flipped on in Nightly, and it didn't make a difference, so I guess that's not it: I kinda took a stab in the dark since you indicted the problem might have gone (may 2019), but now it resurfaced

emilio commented 4 years ago

This is not related to non-native theming, fwiw, just to how we deal with dynamic changes to styles in presence of this rule: https://github.com/w3c/csswg-drafts/issues/4777.

So this is a Firefox bug triggered by the injection of the stylesheet here, but that being said, the add-on is wrong.

Its approach to inject the sanitized style doesn't work with shadow DOM at all. It should use user styles instead, which will also prevent this bug from triggering, coincidentally.

mlgualtieri commented 4 years ago

So this is a Firefox bug triggered by the injection of the stylesheet here, but that being said, the add-on is wrong.

Its approach to inject the sanitized style doesn't work with shadow DOM at all. It should use user styles instead, which will also prevent this bug from triggering, coincidentally.

I looked at user stylesheets when I was writing the extension. They unfortunately do nothing to prevent CSS Exfil (at least that was the case 2 years ago). While the CSS rules could be sanitized, it did not prevent the network connections from being made to the exfil url, which is what the extension is trying to prevent.

I'm aware the lines that you linked to cause the issue, but they are also required to prevent the browser from preloading remote URL's referenced by CSS before they can be sanitized.

The way the extension works is that a stylesheet is injected as soon as possible into the DOM to prevent the exfil of any data by blocking all styles, then all rules are sanitized, and the original blocking CSS stylesheet is removed. I haven't found any other method of preventing the browser from preloading remote resources before sanitization.

emilio commented 4 years ago

@mlgualtieri how don't they prevent the requests from going through? An !important user rule overrides non-!important author one. And at least on Firefox we should only request the one that ends up applying.

Thorin-Oakenpants commented 4 years ago

@emilio This is the relevant css-exfil bugzilla 1531601 if you're interested

mlgualtieri commented 4 years ago

FYI - I coded up a test that uses insertCSS to apply a user style sheet that contains the 'load blocking' CSS rules. Coding it in this way seems to remove the Firefox UI glitches described in this bug. But also, as predicted, the CSS is applied too late in the page load and allows exfil data to leak.

I'll play with this a bit more to see if I can find a happy medium.

mlgualtieri commented 4 years ago

Actually I may have spoken too soon. I had an idea right after I posted the last message, and I think I have it working properly now. Needs more testing, but it's looking positive. Blocks data exfil and prevents the UI glitches.

mlgualtieri commented 4 years ago

I believe this is fully resolved with today's 1.1.0 release. This release changes how the CSS load blocking stylesheet is applied and removed to the page in Firefox, which appears to fix this flaw.

emilio commented 4 years ago

FWIW, the underlying firefox bug should be fixed in FF 80: https://bugzilla.mozilla.org/show_bug.cgi?id=1645773

mlgualtieri commented 4 years ago

Oh well, haha. At least it's also fixed in the plugin too!