Page MenuHomePhabricator

CheckUser should use methods from the IP class to validate IPs and CIDR ranges
Closed, ResolvedPublic

Description

Marking as security for now since it contains an IP range.

I was able to block the range 2600:7400:0:0:0:0:0:0/28 (or as I had entered it in the block form, 2600:7400::/28), but when I tried to run a check on that same range, I got the "invalid IP" error (I tried both forms and got the same result). It is a valid /28 range of IPv6 addresses, so I presume there is something wrong with the CheckUser code. I will investigate and propose a patch myself soon.

Event Timeline

Turns out the reason for this is that $wgBlockCIDRLimit by default allows blocking IPv6 in up to /19, but $wgCheckUserCIDRLimit will not allow checking ranges wider than /32. Not knowing what the reason behind this is, I will leave it as is for now.

However, as I was investigating this I noted the CheckUser uses a regex-based method to identify IPs and ranges, which is bad because we already have methods to do this in MediaWiki core. So I will submit a patch to fix that.

Huji renamed this task from Certain IPv6 ranges cannot be checked but can be blocked to CheckUser should use methods from the IP class to validate IPs and CIDR ranges.Jul 27 2017, 11:29 PM
Huji removed a project: acl*security.
Huji edited subscribers, added: gerritbot; removed: GerritBot.

So apparently Gerritbot cannot send notifications here due to the Custom Policy (which by the way can be removed). So I am adding a link to the patch manually: https://gerrit.wikimedia.org/r/#/c/368327/

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 16 2017, 10:52 PM

Change 368327 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Use methods from the IP class to validate IPs and CIDR ranges

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

Huji removed a project: Patch-For-Review.

Re-opening it as the problem persists despite the patch. A range like 2A01:4F8:0:0:0:0:0:0/29 works when you block it, but gives an "Invalid IP address" error when you try to check it with CU. I think the :4F8: part is what is throwing it off (it is only three characters long, not 4).

I think it don't pass the part ( IP::isIPv6( $ip ) && $range < $wgCheckUserCIDRLimit['IPv6'] ) ) since 29 < 32 (range is too wide). Is the bug in the block stuffs instead?

Huji closed this task as Resolved.EditedOct 10 2017, 4:32 PM

Actually, it is not a bug; the IPv6 default for maximum range size is different for blocks and CheckUser. I don't know why (and I wish they were not the same) but there is no issue with the code.

CheckUser's config: $wgCheckUserCIDRLimit['IPv6'] = 32
Block config: $wgBlockCIDRLimit['IPv6'] = 19