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

The shadowroot attribute gets special treatment, but the new shadowrootmode attribute should too #767

Closed
mfreed7 opened this issue Feb 11, 2023 · 4 comments

Comments

@mfreed7
Copy link

mfreed7 commented Feb 11, 2023

See the code:

https://github.com/cure53/DOMPurify/blob/main/src/purify.js#L1465

The shadowroot attribute, which triggers "old-style" declarative shadow DOM, gets special treatment. See this issue for context: #584

Recently, the declarative Shadow DOM feature has had renewed interest, and as part of that conversation, the shadowroot attribute was renamed to shadowrootmode. Chrome has shipped this new attribute in M111. Chrome also will continue supporting the old shadowroot attribute for some time, as developers migrate.

I believe DOMPurify should be updated to look for both shadowroot and shadowrootmode.

@securityMB

@cure53
Copy link
Owner

cure53 commented Feb 12, 2023

Aye, thanks - that proposal looks good to me and and worth a change.

So, the changed syntax should be this? if (ALLOWED_ATTR.shadowroot || ALLOWED_ATTR.shadowrootmode) { ?

@mfreed7
Copy link
Author

mfreed7 commented Feb 12, 2023

Aye, thanks - that proposal looks good to me and and worth a change.

So, the changed syntax should be this? if (ALLOWED_ATTR.shadowroot || ALLOWED_ATTR.shadowrootmode) { ?

Looks right to me!

@cure53
Copy link
Owner

cure53 commented Feb 13, 2023

Cool, thanks - will add :)

cure53 added a commit that referenced this issue Feb 13, 2023
feat: added better check for shadowrootmod
cure53 added a commit that referenced this issue Feb 13, 2023
see #761

feat: added better check for shadowrootmod
feat: added ALLOW_SELF_CLOSE_IN_ATTR tag
test: added test case
@cure53
Copy link
Owner

cure53 commented Feb 13, 2023

This should be fixed now!

@cure53 cure53 closed this as completed Feb 13, 2023
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

No branches or pull requests

2 participants