WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#23350 closed defect (bug) (invalid)

Pingback Denial of Service Fix - filter_var based IP validation

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Pings/Trackbacks Keywords:
Focuses: Cc:

Description

Just a small improvement upon review of [23330] (some points were not clear to me, especially colons in hostnames and trimming of the hostnames for dots; left them as-is).

Also used strcasecmp.

BTW, what about #4137, can it be closed now?

Attachments (1)

23350.patch (1.7 KB) - added by hakre 5 years ago.

Download all attachments as: .zip

Change History (5)

@hakre
5 years ago

#1 @nacin
5 years ago

This was an SSRF fix, not directly a DoS fix. #4137 remains valid.

I generally just opt for what was done here, but sure, strcasecmp() is fine.

filter_var() was deliberately avoided because it has numerous bugs in 5.2.x and 5.3.x, in particular IDN domains but other bugs (IIRC) as well.

Colons in hostnames and trimming the hostname for dots were both deliberate. I'll publicly commit our extensive unit tests in the near future.

At a glance , your isset() appears proper.

#2 @nacin
5 years ago

In the future, if you have specific questions about a security fix, you can also email security@…, in case you have found something sensitive in nature.

#3 @hakre
5 years ago

Yes, the isset was missing too, forgot to mention. filter_var worked well for my tests, I still can run more tests against PHP 5.2, but I would need to know into which concrete bugs you run, especially with IPv4 validating which is used in the patch. I did not found any issues so far.

#4 @chriscct7
3 years ago

  • Keywords has-patch removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
  • Version changed from 3.5.1 to 3.5

filter_var() when run on 5.2.x and 5.3.x can't validate some of the newer custom top level domains, as Nacin mentioned. The affected area no longer exists anyways. Closing as invalid for this reason.

Note: See TracTickets for help on using tickets.