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

Enable validation of service workers #328

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

aselbie
Copy link
Contributor

@aselbie aselbie commented Jun 13, 2024

Updates CV to validate service workers, which were previously ignored.

In order to achieve this, this PR includes the following:

  • Service workers must now how valid worker CSPs
  • Service workers must be cacheable
  • Code Verify is now able to effectively handle dynamic strings. This is done via AST parsing which makes it resilient to all sorts of string manipulation shenanigans and other attack vectors.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 13, 2024
Copy link
Contributor

@rich-hansen rich-hansen 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, but let's wait for Ezz to give it a once over as well.

@@ -614,3 +615,15 @@ chrome.runtime.onMessage.addListener(request => {
}
}
});

function hasServiceWorkerHeader(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we name this hasVaryServiceWorkerHeader for accuracy?

node.start,
);
if (before === DYNAMIC_STRING_MARKER) {
ranges.push(node.start + 1, node.end - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be helpful to comment that this the string excluding the left/right quotes, so that the empty string is what is validated.

@ezzak ezzak merged commit 63d38db into facebookincubator:main Jun 14, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants