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

Security: Comments can still be used to smuggle arbitrary CSS #1794

Closed
8 tasks done
hackvertor opened this issue Nov 3, 2021 · 9 comments
Closed
8 tasks done

Security: Comments can still be used to smuggle arbitrary CSS #1794

hackvertor opened this issue Nov 3, 2021 · 9 comments
Labels
fixed issue has been addressed

Comments

@hackvertor
Copy link

Prerequisites

I tried to reproduce the issue when...

  • uBO is the only extension
  • uBO with default lists/settings
  • using a new, unmodified browser profile

Description

This bug is related to #1693 and was marked fixed. However, it's still possible to smuggle arbitrary CSS through filter selectors. The following filter list causes a CSS injection:

##input,input/*
##input[x="*/{}*{background-color:red;}"]

The {} is used to make the CSS parser drop everything before as an invalid selector, this then allows you to define your own selector, in this case I make the background red but you could change it to use background URLs etc.

A specific URL where the issue occurs

https://portswigger-labs.net/

Steps to Reproduce

  1. Create a filter in my filters:
##input,input/*
##input[x="*/{}*{background-color:red;}"]
  1. Visit https://portswigger-labs.net/ and the background should appear red.

Expected behavior

The background should not appear red and arbitrary CSS should not be allowed to be injected.

Actual behavior

The filter injects CSS onto the page and the background goes red.

uBlock Origin version

1.38.4

Browser name and version

Chrome 95.0.4638.69

Operating System and version

MacOS 10.15.7

@gwarser
Copy link

gwarser commented Nov 3, 2021

Invalid generic cosmetic filter in user-filters: ##input,input/*

Is logged in 1.38.7b12

@uBlock-user uBlock-user added the something to address something to address label Nov 3, 2021
@uBlock-user
Copy link
Contributor

and the background goes red.

Can't repro that on Firefox.

@uBlock-user uBlock-user added the Chromium specific to Chromium/Chrome label Nov 3, 2021
@gorhill
Copy link
Member

gorhill commented Nov 3, 2021

The commit gorhill/uBlock@4f92338 made the issue here go away.

Also, there is no need to use words like smuggle or arbitrary when it's already allowed by design to apply background-color:red with a cosmetic filter like:

*#$#* { background-color: red }

@hackvertor
Copy link
Author

@gorhill The background colour was only to demonstrate the issue. It's possible to use background urls too. Which isn't possible with a cosmetic filter right? That's why I said arbitrary CSS because normally this wouldn't be allowed.

@gorhill
Copy link
Member

gorhill commented Nov 3, 2021

It's possible to use background urls too

Please provide a proof of concept.

@hackvertor
Copy link
Author

Sure ...

##input,input/*
##input[x="*/{}*{background:url(https://hackvertor.co.uk/images/logo.gif)}"]

@gorhill
Copy link
Member

gorhill commented Nov 3, 2021

Thanks, I can reproduce. This commit was specifically looking for opening comment in style declarations, while in the current case the opening comment is outside a style declaration.

The filter ##.fail,.fail/* (test page) no long work because now uBO detects that this filter can't be injected in a declarative manner in a stylesheet, the browser rejects the declaration because of the dangling /*.

Currently when this happens, uBO tries to compile the filter as a procedural one, but end up rejecting it because implicitly global procedural filters are forbidden.

So I tried *##.fail,.fail/* which render the filter valid as an explicit global filter, but due to the fact that the filter is compiled as a procedural one, it will never be injected in a style sheet, which was the root issue here.

So it appears by chance this commit took care of the issue here.

@uBlock-user uBlock-user removed the Chromium specific to Chromium/Chrome label Nov 3, 2021
@gorhill gorhill closed this as completed Nov 3, 2021
@uBlock-user uBlock-user added fixed issue has been addressed and removed something to address something to address labels Nov 3, 2021
@gorhill
Copy link
Member

gorhill commented Dec 6, 2021

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

No branches or pull requests

5 participants
@gorhill @gwarser @hackvertor @uBlock-user and others