justify or revert addition of "position" to allowed CSS list

[jkl-ds]

d580039 added "position" to the default allowed CSS list without obvious justification.

The "position" property previously had been disallowed, presumably because of the potential to use it to create a misleading interface by overlaying malicious content.

http://www.technicalinfo.net/papers/Phishing.html

Several methods exist for Phishers to override displayed content. One of the most popular methods of inserting fake content within a page is to use the DHTML function - DIV. The DIV function allows an attacker to place content into a “virtual container” that, when given an absolute position and size through the STYLE method, can be positioned to hide or replace (by “sitting on top”) underlying content.

Please:

1. justify the decision to allow "position" or revert the change
2. add justifications to all additions in the future and do so in smaller commits
3. advise whether a CVE will be issued

[mganss]

Thanks for pointing this out. Embarrassingly, the list of CSS properties in the README wasn't updated, either 😳

I'm not sure how to proceed re default CSS properties. The goal of HtmlSanitizer is to prevent XSS. XSS through standard CSS is impossible so we could allow all standard CSS properties. This is what's currently happening. OTOH we might want to try and minimize other kinds of wrongdoing like visually hiding elements etc. If that was the goal we'd have to disallow a lot of CSS properties (all the color ones, visibility etc etc).

[jkl-ds]

Concern over the position property comes from a "sandbox" security model of page content, where the goal is to allow an untrusted party to supply markup for a given region of the page. The untrusted markup should not affect anything else on the page by:

• running scripts
• modifying style rules
• breaking page rendering via plaintext tags
• creating overlays
• hijacking events which use CSS selectors

In this model, color and visibility attributes are permitted even though they could be used to make misleading content, because the security concern is about how the untrusted markup interacts with the rest of the page.

[mganss]

So your suggestion is to exclude only the position property and allow all other standard CSS properties by default?

[jkl-ds]

Yes.