Page MenuHomePhabricator

ip_in_range should accept explicit range notation
Open, Needs TriagePublic

Description

On Logstash, there are several entries for filters failing with

AbuseFilter parser error for filter xxx: Invalid IP range yyy

Examining those filters reveals that all such invalid ranges are explicit ranges, i.e. 1.2.3.4 - 1.2.3.55. AbuseFilter used to accept these ranges in the past, as IP::isInRange can handle them. However, rEABF7fade990d26c79fb6acc100653bcf639f724569a added IP range validation to avoid invalid notations to be used. The downside is that validation is performed with IP::isValidRange, which only checks for ranges in CIDR notation. Given that several filters use explicit notation, and it's a good idea to keep it valid as it's easier to use, we should somehow change the validation to accept these.
AFAICS, the IP class doesn't provide a method to validate explicit ranges, so it's probably worth adding one.

Event Timeline

Daimona created this task.Mar 12 2019, 8:58 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 12 2019, 8:58 AM
Huji added a subscriber: Huji.Mar 12 2019, 11:07 PM

Perhaps the IP class should have a method like rangeToCIDR()

Well, for the specific goals of this task, we'd need a method like isValidRange but for explicit ranges. Maybe something like isValidExplicitRange.

Change 498201 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] IP: Add isValidExplicitRange function

https://gerrit.wikimedia.org/r/498201

Change 498204 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Allow single IPs and explicit ranges in ip_in_range

https://gerrit.wikimedia.org/r/498204

Daimona claimed this task.Mar 21 2019, 6:57 PM
Daimona moved this task from Backlog to Under review on the User-Daimona board.