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

Update CSS filter #998

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Update CSS filter #998

merged 2 commits into from
Nov 15, 2024

Conversation

torusrxxx
Copy link
Contributor

Add dominant-baseline

Copy link
Contributor

@ArneBab ArneBab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Besides: you don’t have to make the PRs this small — just a balance that’s easy to review. I can review 5-6 CSS properties in one sitting, the PR just shouldn’t take more than 10 minutes to review (if possible! If that’s not possible, because the change is intrinsically hard, that’s OK!) and when it’s reviewed the changes should only be to address review comments, avoiding to add new stuff to review (that could spark more review comments). Basically making it easy to progress to a state where the PR can be approved and later merged.

@ArneBab ArneBab merged commit e496dc1 into hyphanet:next Nov 15, 2024
1 check passed
@ArneBab
Copy link
Contributor

ArneBab commented Nov 15, 2024

Merged — thank you!

@ArneBab
Copy link
Contributor

ArneBab commented Nov 19, 2024

@torusrxxx if you want to do a directed search for missing features, you could see what’s missing for the defcon page to render correctly. If you have pyFreenet3 installed (pip install pyFreenet3), you can create a testcopy with copyweb -d defcon https://defcon.org — then just insert with freesitemgr to see what’s missing from the CSS.

@torusrxxx
Copy link
Contributor Author

I know a lot of important missing features, but most of them require significant changes to how the filter works.

First one is media query. A lot of media types were deprecated, only screen, print and all remains. But Freenet keeps mentioning it everywhere. I think I need to remove all these things entirely. Now media query is mostly used for testing media features, important for mobile, but this is currently not supported, and Freenet's code for media types can't be used.

Next one is CSS variables. The use of CSS variables has become widespread, especially for dark mode support. But CSS variables are hard to filter, because it can be of any type with any value and defined anywhere. Since we filter all CSS files, it's still possible to ensure all CSS variables does not contain external links. However it also means almost all of our filter code becomes useless. Currently color: 3pt; is thought as unsafe, but with CSS variables, color: var(--color); --color: 3pt; is possible, won't be filtered away. So we will have to think whether we consider color: 3pt; safe (I think it is the case). Then lots of code are no longer needed and color: 3pt; will be accepted because color: var(--color); can already circumvent this filtering.

A third one is font awesome. A lot of places use these nice icons. This requires filters for font files. The threat model for font files is yet unknown. There are many known exploits targeting color profiles, fonts like this one for Windows that are already patched, but it's still possible for the user to run an unpatched operating system. It's not possible to look for every exploits of every operating system. So, either we have to ask users to use patched operating systems, or we can't support font awesome. The same is also true for WebP, where we have a known exploit for lossless encoding that is already patched, but we still block all WebP images with lossless encoding because we aren't sure whether the user patch the browser and operating system properly.

@ArneBab
Copy link
Contributor

ArneBab commented Nov 20, 2024

Variables may be hard, yes. For font awesome (and other fonts) the issue is clear, though: these are dangerous, because they can be executables doing anything.

Where’s the harm from continuing to support many media types? We have pretty old sites. Media queries for mobile features look orthogonal to that check, so they can be added.

However they have to be sanitized. Look at how Tor Browser sets fixed sizes to the displayed area. This is necessary to prevent individualized tracking — and the same is true in Freenet: we can’t have pixel-perfect media queries, because these would enable tracking.

@torusrxxx
Copy link
Contributor Author

I don't think fonts are that dangerous. Fonts can contain executable but certainly they cannot do anything they want, for example sending web requests. Otherwise there will be a patch to fix it.

Fonts are optional in web pages (the font awesome icons will be missing), but they are essential in PDF. PDF files will be broken if the embedded fonts are removed. There are many libraries to read and write PDF, I think it will be possible to filter PDF just like HTML one day.

@ArneBab
Copy link
Contributor

ArneBab commented Nov 20, 2024

The risk is that such a patch may be too late: if they leak privacy for some time, the damage is done and cannot be repaired. That’s why we usually err on the side of privacy. We’re not perfect, but we try to avoid risks that could be higher than the reward.

That said: limited PDF support may be possible (most tools do not support it completely), but it’s a huge task that needs careful checking.

@torusrxxx
Copy link
Contributor Author

Another off topic question: Is there any plan or patch to support SVG? If there is, I would like to see whether I can help on that.

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

Successfully merging this pull request may close these issues.

2 participants