Allow picture, source, srcset, sizes with a dedicated srcset scrubber - #303
Open
kataokatsuki wants to merge 1 commit into
Open
Allow picture, source, srcset, sizes with a dedicated srcset scrubber#303kataokatsuki wants to merge 1 commit into
kataokatsuki wants to merge 1 commit into
Conversation
Add `picture`/`source` to ACCEPTABLE_ELEMENTS and `srcset`/`sizes` to ACCEPTABLE_ATTRIBUTES. `srcset` is a list of "URL descriptor" candidates, not a single URL, so it gets a dedicated `scrub_srcset_attribute` (a new `ATTR_VAL_IS_SRCSET`) that validates each candidate URL with `allowed_uri?` and is fail-closed, rather than recreating the CVE-2024-8372 bypass class. URLs are split on ASCII whitespace per the WHATWG rules, so commas in `data:` URLs survive. part of flavorjones#155
Owner
|
@kataokatsuki Thanks, I'll take some time to review this week. |
Author
|
Thanks, no rush🙏 |
Author
|
@flavorjones friendly bump in case this got buried! |
Owner
|
@kataokatsuki I've spent most of the last few weeks working on security patches, haven't gotten back to this yet but it's definitely on my to-do list. Thanks for your patience. |
Author
|
@flavorjones Thanks for the update, no worries. I appreciate you taking the time. |
Author
|
Sorry for the noise — my last comment here was meant for a different PR. I've deleted it. |
Owner
|
Sorry for the delay here. I've been working on a Loofah security release, but will get to this in the next few days. |
Author
|
Sounds good, thanks! And sorry again for the stray comment the other day. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #155
Adds
picture/sourceandsrcset/sizesto the safelists.pictureis added assource's parent.srcsetis a list of candidates, so this gives itATTR_VAL_IS_SRCSETandscrub_srcset_attributeinstead of treating it as one URL, which would recreate the CVE-2024-8372 bypass class. The scrubber reads each candidate URL token withStringScanner, validates it withallowed_uri?, and drops the whole attribute if any candidate is disallowed; tests cover the safelists,data:URLs with commas, and fail-closed cases forimgandsource.