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

fix(ses): Lockdown under no-unsafe-eval Content-Security-Policy #1360

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

Conversation

kriskowal
Copy link
Member

This change adds a test that verifies that Lockdown runs to completion even in the absence of a usable evaluator.

@kriskowal
Copy link
Member Author

cc @Jack-Works. I’m leaving this in draft because it does not address your concern about Lockdown generating excessive warnings. But, this should subsume all of #1333. I have not rigorously accounted for @mhofman’s concerns in #903. Posted as draft to show progress.

Copy link
Contributor

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

I have not rigorously accounted for @mhofman’s concerns in #903.

I'll probably have to page this back in memory as I do not currently recall my previous investigation and work in this area.

// eslint-disable-next-line @endo/no-polymorphic-call
FERAL_FUNCTION.prototype.constructor('return 1');
} catch (ignore) {
FERAL_EVAL('');
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we also need to test that FERAL_FUNCTION works to make sure the make-evaluator succeeds?

),
constructor => {
try {
constructor('');
Copy link
Contributor

Choose a reason for hiding this comment

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

I may be paranoid, but wouldn't there technically be a way to bypass this by allowing the "empty" / null sha in CSP?

I guess it'd really be the embedder shooting itself in the foot in that case.

Comment on lines +61 to +64
// eslint-disable-next-line no-empty-function
async function unsafeAsyncFunction() {},
// eslint-disable-next-line no-empty-function
async function* unsafeAsyncGeneratorFunction() {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these actually throw synchronously on CSP violation ?

@kriskowal
Copy link
Member Author

This remains in draft because we need to reframe the solution in terms of a lockdown option that expressly avoids any spelling of eval, and attempts to preserve the invariants of lockdown with the assumption that any attempt by guest code to use any spelling of eval will be thwarted by the host.

cc @erights

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