![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 10 years ago
Closed 10 years ago
#37208 closed defect (bug) (fixed)
Comment blacklist can be bypassed with HTML
| Reported by: | cfinke | Owned by: | rachelbaker |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.6 |
| Component: | Comments | Version: | 1.5 |
| Severity: | normal | Keywords: | has-unit-tests has-patch commit |
| Cc: | Focuses: |
Description
If the comment blacklist contains the entry "bannedword", users can still post comments containing (or appearing to contain) "bannedword" by using HTML:
<b>banned</b>word banned<b></b>word banned<b>word</b>
A solution: wp_blacklist_check() could run wp_kses() before looking for blacklist entries. Patch attached.
Attachments (4)
Change History (17)
This ticket was mentioned in Slack in #core-comments by rachelbaker. View the logs.
10 years ago
#5
@
10 years ago
- Keywords has-unit-tests commit added; needs-unit-tests removed
- Milestone Future Release → 4.6
In 37208.2.diff I added some basic unit tests for wp_blacklist_check including test_should_return_true_when_content_with_html_matches_blacklist_keys
#8
follow-up:
↓ 9
@
10 years ago
@rachelbaker @cfinke Should this just use wp_strip_all_tags() instead of wp_kses()? KSes is designed to limit to a whitelist, if you want no tags, might as well just strip them entirely.
#9
in reply to: ↑ 8
@
10 years ago
Replying to dd32:
@rachelbaker @cfinke Should this just use
wp_strip_all_tags()instead ofwp_kses()? KSes is designed to limit to a whitelist, if you want no tags, might as well just strip them entirely.
I wasn't aware of wp_strip_all_tags(), but it does seem more appropriate.
#10
@
10 years ago
- Keywords needs-patch added; has-patch commit removed
- Resolution fixed
- Status closed → reopened
#11
@
10 years ago
- Keywords has-patch added; needs-patch removed
@dd32 Thanks for catching this. Added 37208_strip_tags.diff patch that uses wp_strip_all_tags()
#12
@
10 years ago
- Keywords commit added
37208_strip_tags.diff looks good, passes unit tests, works.
Added unit test