Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#11833 closed defect (bug) (invalid)

bizarre behavior in the comment form sanitization

Reported by: denis-de-bernardy's profile Denis-de-Bernardy Owned by: ryan's profile ryan
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Security Keywords:
Focuses: Cc:

Description

Found this while trying to break the comment form's multiple link regex (#11830):

<a
href = http://foo.com

<a
href = http://foo.com

>test</a>

when I look into my post's code, I get:

<p><a href = <a  href="http://foo.com" rel="nofollow">http://foo.com</a></p>
<p><a href = <a  href="http://foo.com" rel="nofollow">http://foo.com</a></p>
<p>>test</p>

on the plus side, the nofollow regex works. but those extra < and > should have been html encoded.

Change History (17)

#1 @miqrogroove
15 years ago

Is this reproducible with the default theme? Is it version 3.0 only?

#2 @Denis-de-Bernardy
15 years ago

this is when logged in, btw. as a visitor, I ultimately get:

<p>&lt;a<br />
href = <a  href="http://foo.com" rel="nofollow">http://foo.com</a></p>
<p><a  href="http://foo.com" rel="nofollow">test</a></p>
<p>test4</p>

better, but it does suggest there might be bugs in kses.

@miqrogroove: using 3.0 with my theme and a bunch of plugins active, but nothing that filters anything comments-related.

#3 @miqrogroove
15 years ago

I'm not sure kses is applied at all when you have unfiltered_html caps? The output goes through many many other filters.

#4 @Denis-de-Bernardy
15 years ago

see also this wonderful use case:

http://core.trac.wordpress.org/ticket/11830#comment:4

<
a	
<a	
	href	=	
	http://foo:bar@foo.com#?#

<a
	href	=	http://foo.com?\"'

>test</a>

test6

test5

the stripslashed and filtered result as a visitor:

<
a	
<a	
	href	=	
	http://foo:bar@foo.com#?#

<a>test</a>

test6

test5

<p><<br />
a<br />
<a<br />
	href	=<br />
	<a  href="http://foo:bar@foo.com#?#" rel="nofollow">http://foo:bar@foo.com#?#</a></p>
<p><a>test</a></p>
<p>test6</p>
<p>test5</p>

#5 @Denis-de-Bernardy
15 years ago

err, I'm sorry, that actually is without filters, and then with filters.

#6 @miqrogroove
15 years ago

I really can't get this to happen on 2.9.1 with default theme and no plugins. I get:

<p>&lt;<br />
a<br />
&lt;a<br />
	href	=<br />
	<a href="http://foo:bar@foo.com#?#" rel="nofollow">http://foo:bar@foo.com#?#</a></p>
<p><a>test</a></p>
<p>test6</p>
<p>test5</p>

Needs to be narrowed down to a version or a customization at least.

#7 @Denis-de-Bernardy
15 years ago

yaya, that's in the comment itself when it gets displayed. my dump is looking at it before we count the links, in check_comment(), which is the stage where we're supposed to filter out comment spam.

adding some null chars in there yields some interesting side effects, btw.

#8 @miqrogroove
15 years ago

<a>test</a> really shouldn't bother anyone, which is why the filter is for href.

#9 @miqrogroove
15 years ago

Hey Denis, it looks like one of us managed to break the comment feed on my test site. I'll find out what happened and post some details for this ticket :) If the bug isn't confined to input issues then I'll open a third ticket specifically for the feed system.

#10 @miqrogroove
15 years ago

Looks like it was me -_-

The XSS Cheat Sheet from ha.ckers.org caused the feed validator to throw "undefined entity".

On to debugging...

#11 @miqrogroove
15 years ago

I submitted the feed issue to the security list so it can be discussed privately.

#12 @hakre
15 years ago

Related: #12284

#13 @nacin
15 years ago

I've closed #11830. Is modifying the regex as described there (we have at least three options) fine for this ticket?

#14 @miqrogroove
15 years ago

I still don't see a bug here. What's the question exactly?

#15 @nacin
15 years ago

  • Milestone changed from 2.9.3 to 3.0
<a	
	href	=	
	http://foo:bar@foo.com#?#

<a
	href	=	http://foo.com?\"'

>test</a>

Is technically counted as one link.

We can change the regex in one of three ways (documented in #11830), and all would fix that. I otherwise see no bug.

#16 @miqrogroove
15 years ago

Oh, I thought this was a kses ticket.

Is technically counted as one link.

WordPress only outputs one link, based on my testing.

#17 @nacin
15 years ago

  • Milestone 3.0 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Further testing, I can't come up with anything here.

Note: See TracTickets for help on using tickets.