Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#2130 closed defect (bug) (fixed)

wp_kses kills < !-- more --> from xmlrpc posted entries

Reported by: tsimmons Owned by: ryan
Milestone: Priority: normal
Severity: major Version: 2.0
Component: XML-RPC Keywords: bg|has-patch bg|commit
Focuses: Cc:


I couldn't find the exact point where it happens, but somewhere when XMLRPC posted entries are being processed, the


tags are being stripped which gets rid of the "more..." link and instead displays the entire post on the indexes. I tracked it down to somewhere in the nested functions called in kses.php, somewhere after line 55:

	return wp_kses_split($string, $allowed_html_fixed, $allowed_protocols);

Attachments (4)

kses.php.diff (468 bytes) - added by tsimmons 9 years ago.
Proposed patch
kses.php.2.diff (782 bytes) - added by tsimmons 9 years ago.
New version of suggested patch
kses-comments.diff (668 bytes) - added by skeltoac 9 years ago.
perfect-comments.diff (1.1 KB) - added by skeltoac 9 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 @tsimmons9 years ago

Okay, after more digging, I found it is in wp_kses_split2() on line 101:

if (!preg_match('%^<\s*(/\s*)?([a-zA-Z0-9]+)([^>]*)>?$%', $string, $matches))

This always strips the <!--more--> tags. My regular expression skillz are way down right now (mighty tired after long night ...)

Sorry I can't offer a fix right off ...

comment:2 @tsimmons9 years ago

I can't take credit for this, but how does the fix from http://mu.wordpress.org/forums/topic/450 sound? Add three lines to the function at line 101:

	if (preg_match('%^<!--[^>-]+-->$%', $string))
		return $string;
	# Allow HTML comments

@tsimmons9 years ago

Proposed patch

comment:3 @skeltoac9 years ago

No, this expression will miss valid comments such as this:

@tsimmons9 years ago

New version of suggested patch

comment:4 @tsimmons9 years ago

I have attached a new proposed fix, based on Owen's suggestion in wp-testers. It works for me !!

comment:5 @davidhouse9 years ago

  • Keywords bg|has-patch added

comment:6 @ryan9 years ago

  • Owner changed from anonymous to ryan

comment:7 @masquerade9 years ago

Based on the patch above (untested), the following would pass right through.

&lt;!--something--&gt;&lt;script&gt;malicious code&lt;/script&gt;&lt;!--somethingelse--&gt;

comment:8 @masquerade9 years ago

Er, pretend that didn't get mangled

<!--something--><script>malicious code</script><!--somethingelse-->

comment:9 @ringmaster9 years ago

Can you fix this by adding a ? to the regex in the first replacement block? Like:

return preg_replace('%(<!--.*?-->)|(<'.# EITHER: <

comment:10 @ryan9 years ago

  • Milestone set to 2.0.1

comment:11 @ryan9 years ago

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

(In [3417]) Pass comments through kses. Props tsimmons. fixes #2130 #2167

@skeltoac9 years ago

comment:13 @skeltoac9 years ago

  • Keywords bg|commit added; kses wp_kses xmlrpc more removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

That last patch is broken: the first regex is missing a () so the eval never gets the right string.

perfect-comments.diff fixes that and more carefully filters the contents of the comment. Now, if a comment is left unclosed, it will be closed at the end of the string. Also, nested comments and uneven open/close markers are fixed. Comments are rock-solid.

@skeltoac9 years ago

comment:14 @ryan9 years ago

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

(In [3429]) kses regex tweakage for better comment filtering. fixes #2130

comment:15 @anonymous8 years ago

  • Milestone 2.0.1 deleted

Milestone 2.0.1 deleted

Note: See TracTickets for help on using tickets.