SanitizationHelperTrait: stop requiring unslash for functions that strip slashes#2740
Open
rodrigoprimo wants to merge 1 commit into
Open
SanitizationHelperTrait: stop requiring unslash for functions that strip slashes#2740rodrigoprimo wants to merge 1 commit into
rodrigoprimo wants to merge 1 commit into
Conversation
…rip slashes The following functions implicitly strip backslashes from their input, so a preceding call to `wp_unslash()` (or a similar unslashing function) is not needed for the value to be safe: `esc_url_raw()`, `sanitize_email()`, `sanitize_file_name()`, `sanitize_hex_color()`, `sanitize_hex_color_no_hash()`, `sanitize_html_class()`, `sanitize_mime_type()`, `sanitize_sql_orderby()`, `sanitize_title()`, `sanitize_title_for_query()`, `sanitize_title_with_dashes()`, `sanitize_url()` and `wp_sanitize_redirect()`. They were previously listed in `$sanitizingFunctions`, which made the `WordPress.Security.ValidatedSanitizedInput` sniff incorrectly report a `MissingUnslash` error when one of them was used directly on superglobal data without unslashing it first. Moving them to `$unslashingSanitizingFunctions` fixes this false positive. Fixes 2516.
739cc26 to
1822bbb
Compare
Contributor
Author
|
Rebased and force-pushed without changes to make the Codecov GH action pass. |
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
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.
Description
As discussed in #2516, the
WordPress.Security.ValidatedSanitizedInputsniff incorrectly reported aMissingUnslasherror when one of the following functions was used directly on superglobal data without first callingwp_unslash()(or a similar unslashing function):esc_url_raw()/sanitize_url(): delegates toesc_url(), whose regex[^a-z0-9-~+_.?#=!&;,/:%@$|*'()\[\]\x80-\xff]strips backslashes.sanitize_email(): the regex[^a-zA-Z0-9!#$%&'*+/=?^_`{|}~.-]strips backslashes.sanitize_file_name():\is explicitly included in the$special_charsarray passed tostr_replace().sanitize_hex_color(): returns the input only if it matches^#([A-Fa-f0-9]{3}){1,2}$, otherwise returns nothing.sanitize_hex_color_no_hash(): delegates tosanitize_hex_color().sanitize_html_class(): regex[^A-Za-z0-9_-]strips backslashes.sanitize_mime_type(): regex[^-+*.a-zA-Z0-9\/]strips backslashes.sanitize_sql_orderby(): returns the input only if it matches one of two regexes (neither of which allows backslashes),falseotherwise.sanitize_title_with_dashes(): final regex[^%a-z0-9 _-]strips backslashes.wp_sanitize_redirect(): regex[^a-z0-9-~+_.?#=&;,/:%!*\[\]()@]strips backslashes.sanitize_title(): the function body itself doesn't strip backslashes, but it callsapply_filters( 'sanitize_title', ... )andsanitize_title_with_dashes()is hooked to that filter by default.sanitize_title_for_query(): wrapssanitize_title( $title, '', 'query' ), so same reasoning assanitize_title().All of these implicitly strip backslashes from their output, so requiring a separate unslashing call before them is a false positive as far as I can see. This moves them from
$sanitizingFunctionsto$unslashingSanitizingFunctionsinSanitizationHelperTrait.This change only affects the
WordPress.Security.ValidatedSanitizedInputsniff.To verify that the functions above indeed strip slashes, I used
wp shellto call them with strings containing slashes and checked that the slashes were removed from the returned value.Fixes #2516.
Suggested changelog entry
WordPress.Security.ValidatedSanitizedInput: false positiveMissingUnslashforesc_url_raw(),sanitize_email(),sanitize_file_name(),sanitize_hex_color(),sanitize_hex_color_no_hash(),sanitize_html_class(),sanitize_mime_type(),sanitize_sql_orderby(),sanitize_title(),sanitize_title_for_query(),sanitize_title_with_dashes(),sanitize_url()andwp_sanitize_redirect(), which implicitly strip slashes and therefore do not require unslashing.