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

Address code execution escalation concerns #15

Merged
merged 11 commits into from
Aug 28, 2024
Merged

Address code execution escalation concerns #15

merged 11 commits into from
Aug 28, 2024

Conversation

weizman
Copy link
Collaborator

@weizman weizman commented Aug 27, 2024

Come to think about it, using a CSP header for this feature can potentially introduce security concerns worth reflecting - this PR summarizes important thoughts around it

@weizman
Copy link
Collaborator Author

weizman commented Aug 27, 2024

@yoavweiss I think I'm leaning towards option#2 (meta tag) which is a great sweet spot in the middle between a header and a JS API, because:

  1. It's a better version of the header, because it is more unlikely to inject tags, and even more so to inject them high enough in the initial HTML so that they bypass the RIC dictation of the web app itself
  2. It's a better version of the header, because it's less of an escalation - if attackers can inject HTML, they might as well inject a script or an iframe
  3. It's a better version of the JS API, because it deviates significantly less from the original proposal

That being said, when comparing option#3 to the serviceWorker API, it makes a lot of sense, as the 2 are much alike in terms of power and SOP sensitivity

All in all, a lot to ponder, but what's becoming more clear to me is that option#1 (current proposal's state) is probably the worst one...

Would love to hear your thoughts

@weizman weizman changed the base branch from weizman-sec-priv-con to main August 27, 2024 14:35
Copy link
Collaborator

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

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

LGTM % comment

README.md Outdated
* CON - Even if mitigated, still potentially an escalation, for example if attacker can introduce HTML tags but is blocked from getting them to execute thanks to a strict `script-src` implementation, providing a meta tag with `init-realm` can help them escalate to XSS).
3. API (via JavaScript) - instead of CSP, export a JavaScript API that registeres the script to run within all realms (e.g. `window.onRealmInit('/scripts/init-realm.js')`)
* PRO - This mitigates possibilities for escalations towards code execution, because in order to abuse this feature the attacker must have code execution abilities to begin with
* CON - Potentially allows all scripts in the page to register their own script (although probably not an issue, as long as order of registration is respected)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will allow 3P scripts to register 3P RIC scripts, which can be sensitive.. Maybe we can add some controls in place if that's not desired (e.g. the API is only available for 1P scripts)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about it @yoavweiss, but that's also unprecedented, meaning:

  • The browser does not tell 1p apart from 3p scripts at any scenario (AFAIA), so this will require us to define and introduce new capabilities and checks for being able to generate such information
  • This will introduce new capabilities in the browser for entities hosted within a web app (for example, it can be used to tell whether a script runs within 1p or 3p, which cannot be reliably done today)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a clarification 09ac2c3

@weizman weizman merged commit 307e48f into main Aug 28, 2024
1 check passed
@weizman
Copy link
Collaborator Author

weizman commented Aug 28, 2024

This is an open issue. Continue discussion at #16

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