Ticket #938 (closed defect: fixed)

Opened 4 years ago

Last modified 2 years ago

maximum number of links in comment "exploit"

Reported by: Ozh Assigned to:
Priority: low Milestone:
Component: General Version: 1.5.1
Severity: minor Keywords: has-patch
Cc:

Description

Function 'check_comment' in comment-functions.php counts the number of 'http:' occurences. So a spammer can post as much links as wished if typed this way : <a href="//crapsite.com">link</a>

Attachments

938.diff (0.6 kB) - added by Nazgul on 09/30/06 23:54:22.
938b.diff (0.6 kB) - added by Nazgul on 10/04/06 04:33:31.

Change History

02/21/05 13:46:00 changed by Ozh

  • Patch set to No.

02/24/05 07:55:51 changed by ryan

  • Patch changed from No to Yes.

06/30/05 14:22:52 changed by skippy

  • keywords set to bg|2nd-opinion.
  • owner changed from anonymous to skippy.
  • status changed from new to assigned.
  • priority changed from normal to low.

Should check_comment check non-http links? FTP / https / ??

Is this a big deal?

06/30/05 17:12:44 changed by markjaquith

  • keywords changed from bg|2nd-opinion to bg|2nd-opinion bg|dev-feedback.

well, the //site.com links would be clickable... so they should probably get counted. I don't think https or FTP is going to be a problem, though.

We could count href="http:// and href="// and combine the two, or move to a regex. Also have to consider href='http:// and href='// so it might be time to go the regex route.

Another issue: <a href="http://site.com/">http://site.com/</a> gets penalized twice with the current situation, and if you're using a formatting filter on your comments, those links could get through on their own. Should we be processing the comment text filters before looking for links? That's how it's going to appear on your site...

08/18/05 11:42:30 changed by davidhouse

The quotes are optional in HTML, so don't require their presence. I.e., <a href=http://example.com>blah</a> works in most browsers. I'd suggest checking with:

#href=('|")?(https?:)?//("|')?#

09/30/06 23:54:22 changed by Nazgul

  • attachment 938.diff added.

09/30/06 23:57:18 changed by Nazgul

  • keywords changed from bg|2nd-opinion bg|dev-feedback to 2nd-opinion dev-feedback has-patch.
  • milestone set to 2.1.

This ticket hasn't been updated in ages, but I think it's still valid.

I've created a patch for it which should cover most of the given scenarios, but some feedback would be appreciated.

10/01/06 22:25:17 changed by foolswisdom

  • owner deleted.
  • status changed from assigned to new.

Clearing owner = skippy

10/04/06 04:33:31 changed by Nazgul

  • attachment 938b.diff added.

10/04/06 04:34:51 changed by Nazgul

  • keywords changed from 2nd-opinion dev-feedback has-patch to has-patch.
  • milestone changed from 2.1 to 2.0.5.

Updated patch, based on feedback on IRC:

[06:29] MarkJaquith?: BasB: regarding the \/\/ part (http://) ... you're using pipes to delimit the regex, so you shouldn't have to escape those forward slashes [06:30] MarkJaquith?: oh, and let's make it case insensitive

Also marking this as a 2.0.5 candidate.

10/04/06 04:53:18 changed by markjaquith

  • status changed from new to closed.
  • resolution set to fixed.

(In [4299]) comment link counting improvements from Nazgul. fixes: #938

10/04/06 05:59:38 changed by foolswisdom

[4300] same fix for branches/2.0 -> 2.0.5

11/30/06 19:41:51 changed by

  • milestone deleted.

Milestone 2.0.5 deleted