WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#6642 closed defect (bug) (fixed)

Commenters can break page validation via HTML comments

Reported by: schiller Owned by:
Milestone: 2.6.1 Priority: normal
Severity: normal Version: 2.6
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

As per http://www.w3.org/TR/REC-xml/#sec-comments, XML does not like two dashes (--) in comments, nor does it like comments ending in --->. This should be fixed in kses

Attachments (1)

bug6642.patch (475 bytes) - added by schiller 6 years ago.
Patch for kses, prevents adjacent hyphens in a HTML/XML comment

Download all attachments as: .zip

Change History (17)

schiller6 years ago

Patch for kses, prevents adjacent hyphens in a HTML/XML comment

comment:1 schiller6 years ago

  • Cc rubys@… added

comment:2 Viper007Bond6 years ago

wptexturize() converts a double dash into –, so no problems there.

comment:3 schiller6 years ago

Can you clarify this? When is wptexturize() called? Is this something that has changed since WP 2.3.3?

comment:4 Viper007Bond6 years ago

  • Milestone 2.7 deleted
  • Resolution set to worksforme
  • Status changed from new to closed

No, wptexturize() has been around since at least version 1.5. All comments and posts are run through it by default before being displayed.

Log out and make a comment like this on your blog:

This is a -- test comment over here --->

It will display at this valid XHTML:

This is a — test comment over here —>

Closing as worksforme.

comment:5 Viper007Bond6 years ago

Oh, and to answer your "When is wptexturize() called?" question, look at /wp-includes/default-filters.php. You'll find this line in it:

add_filter('comment_text', 'wptexturize');

comment:6 schiller6 years ago

Actually I had already confirmed this was indeed a problem - someone was logged out and made the following comment on my WP 2.3.3 blog:

Comment: <!-- foo -- bar -->

And it resulted in a Yellow Page of Death when rendered as XHTML. That's why I dug through and came up with this 2-line patch for kses.

Note that the comment stays hidden i.e. it actually stays a HTML comment it doesn't get escaped to be

Comment: &lt;!-- foo -- bar --&gt;

I do not have the "WordPress should correct invalidly nested XHTML automatically" checkbox checked (Options > Writing). Can you describe the settings on your blog that relate to translating markup?

comment:7 Viper007Bond6 years ago

  • Keywords needs-patch added; xhtml kses removed
  • Milestone set to 2.7
  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Version set to 2.5

Okay, well that's an entirely different issue. ;)

Confirmed that no-access users can post HTML comments, something that they shouldn't be able to do IMO. It's specifically allowed in the code though, so then I guess we should just make sure it doesn't break validation.

comment:8 Viper007Bond6 years ago

  • Summary changed from kses should not allow multiple hyphens in comments to Commenters can break page validation via HTML comments

comment:9 follow-up: schiller6 years ago

Ok, thanks - I should have clarified between the two different types of comments ;)

I did attach a patch for this bug - does it need to get reviewed or something? (Just curious about your addition of the 'needs-patch' keyword)

comment:10 in reply to: ↑ 9 Viper007Bond6 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Replying to schiller:

I did attach a patch for this bug - does it need to get reviewed or something? (Just curious about your addition of the 'needs-patch' keyword)

Sorry, force of habit and I thought your patch merely removed all double dashes. It was in the wee hours of the morning and I didn't realize your patch was specifically targeted at HTML comments. My apologies.

Switched to the "has-patch" tag. :)

comment:11 azaozz6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [8382]) Prevent adjacent hyphens in a HTML/XML comment. Fixes #6642 for trunk. Props schiller.

comment:12 azaozz6 years ago

  • Milestone changed from 2.7 to 2.6.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-open for 2.6.1

comment:13 azaozz6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [8383]) Prevent adjacent hyphens in a HTML/XML comment. Fixes #6642 for 2.6.1. Props schiller.

comment:14 codedread4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 2.5 to 2.9.1

This appears broken again in WP 2.9.1 (though I did verify my fix appears in kses.php still). No idea why it's happening.

comment:15 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
  • Version changed from 2.9.1 to 2.6

Since this ticket was marked as fixed for a shipped milestone, please open a new ticket and reference this one.

comment:16 rubys4 years ago

  • Cc rubys@… removed
Note: See TracTickets for help on using tickets.