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

test: expand the browser smoke test to break when lockdown doesn't work under CSP denying eval #2362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naugtur
Copy link
Member

@naugtur naugtur commented Jul 18, 2024

This test will make sure we don't start relying on direct eval for lockdown itself.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

ELI5

@kriskowal
Copy link
Member

The test in question only runs lockdown and does not attempt to execute confined code. So, it may as well perform that test under a prohibitive Content-Security-Policy.

This will interact in interesting ways with using eval for feature-detection. We should go on to furthermore verify that Lockdown was effective for the questionable cases (like %AsyncGeneratorPrototype%)

@erights erights self-requested a review July 18, 2024 23:22
Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

@naugtur , I know so little about CSP that you should ask someone else. I'm unassigning myself as a reviewer.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

Oh. I cannot unassign myself. Nevertheless...

@naugtur
Copy link
Member Author

naugtur commented Aug 2, 2024

The relevant information here is that CSP denies all evaluators by default (causes them to throw) and lockdown already supports that. If the feral Function cannot be used as an evaluator, the fragment of lockdown that uses eval('async function*(){}') to get the various function type constructors is skipped because there's no need to tame them.

This test change is intended as a regression check in case we use the same trick elsewhere - in places where it would be needed for lockdown under CSP as well.

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.

3 participants