From 4134a0b70fee4478ae21bda54b0af9b67c7a354e Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 6 May 2013 10:27:46 -0400 Subject: [PATCH] API: Fix IPv6 handling in list=blocks The current handling of the bkip parameter assumes IPv4, and breaks for IPv6 CIDR ranges (it also isn't working right for IPv6 non-CIDR, but not in an obvious way). This rewrite handles IPv6 correctly. It also necessarily adds validation for the bkip parameter, which would formerly return (not very sensible) results when passed invalid values. Bug: 48129 Change-Id: I02471bb32c3a217004d07a79d9f98b62133b31ef --- RELEASE-NOTES-1.22 | 4 +++ includes/api/ApiQueryBlocks.php | 52 ++++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index 841de3530f..52ca63f86c 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -174,6 +174,10 @@ production. image (sha1 and timestamp). * (bug 49239) action=parse now can parse in preview and section preview modes. * (bug 49259) action=patrol now accepts revision ids. +* (bug 48129) list=blocks&bkip= now correctly handles IPv6 CIDR ranges and + honors $wgBlockCIDRLimit. Note any clients passing invalid values to bkip + will now receive an error, rather than the previous behavior listing all + user blocks. === Languages updated in 1.22=== diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php index 881269fe02..e3c27f5e2a 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -91,18 +91,30 @@ class ApiQueryBlocks extends ApiQueryBase { $this->addWhereFld( 'ipb_auto', 0 ); } if ( isset( $params['ip'] ) ) { - list( $ip, $range ) = IP::parseCIDR( $params['ip'] ); - if ( $ip && $range ) { - // We got a CIDR range - if ( $range < 16 ) { - $this->dieUsage( 'CIDR ranges broader than /16 are not accepted', 'cidrtoobroad' ); - } - $lower = wfBaseConvert( $ip, 10, 16, 8, false ); - $upper = wfBaseConvert( $ip + pow( 2, 32 - $range ) - 1, 10, 16, 8, false ); + global $wgBlockCIDRLimit; + if ( IP::isIPv4( $params['ip'] ) ) { + $type = 'IPv4'; + $cidrLimit = $wgBlockCIDRLimit['IPv4']; + $prefixLen = 0; + } elseif ( IP::isIPv6( $params['ip'] ) ) { + $type = 'IPv6'; + $cidrLimit = $wgBlockCIDRLimit['IPv6']; + $prefixLen = 3; // IP::toHex output is prefixed with "v6-" } else { - $lower = $upper = IP::toHex( $params['ip'] ); + $this->dieUsage( 'IP parameter is not valid', 'param_ip' ); + } + + # Check range validity, if it's a CIDR + list( $ip, $range ) = IP::parseCIDR( $params['ip'] ); + if ( $ip !== false && $range !== false && $range < $cidrLimit ) { + $this->dieUsage( "$type CIDR ranges broader than /$cidrLimit are not accepted", 'cidrtoobroad' ); } - $prefix = substr( $lower, 0, 4 ); + + # Let IP::parseRange handle calculating $upper, instead of duplicating the logic here. + list( $lower, $upper ) = IP::parseRange( $params['ip'] ); + + # Extract the common prefix to any rangeblock affecting this IP/CIDR + $prefix = substr( $lower, 0, $prefixLen + floor( $cidrLimit / 4 ) ); # Fairly hard to make a malicious SQL statement out of hex characters, # but it is good practice to add quotes @@ -295,6 +307,7 @@ class ApiQueryBlocks extends ApiQueryBase { } public function getParamDescription() { + global $wgBlockCIDRLimit; $p = $this->getModulePrefix(); return array( 'start' => 'The timestamp to start enumerating from', @@ -302,8 +315,12 @@ class ApiQueryBlocks extends ApiQueryBase { 'dir' => $this->getDirectionDescription( $p ), 'ids' => 'List of block IDs to list (optional)', 'users' => 'List of users to search for (optional)', - 'ip' => array( 'Get all blocks applying to this IP or CIDR range, including range blocks.', - 'Cannot be used together with bkusers. CIDR ranges broader than /16 are not accepted' ), + 'ip' => array( + 'Get all blocks applying to this IP or CIDR range, including range blocks.', + "Cannot be used together with bkusers. CIDR ranges broader than " . + "IPv4/{$wgBlockCIDRLimit['IPv4']} or IPv6/{$wgBlockCIDRLimit['IPv6']} " . + "are not accepted" + ), 'limit' => 'The maximum amount of blocks to list', 'prop' => array( 'Which properties to get', @@ -384,10 +401,19 @@ class ApiQueryBlocks extends ApiQueryBase { } public function getPossibleErrors() { + global $wgBlockCIDRLimit; return array_merge( parent::getPossibleErrors(), $this->getRequireOnlyOneParameterErrorMessages( array( 'users', 'ip' ) ), array( - array( 'code' => 'cidrtoobroad', 'info' => 'CIDR ranges broader than /16 are not accepted' ), + array( + 'code' => 'cidrtoobroad', + 'info' => "IPv4 CIDR ranges broader than /{$wgBlockCIDRLimit['IPv4']} are not accepted" + ), + array( + 'code' => 'cidrtoobroad', + 'info' => "IPv6 CIDR ranges broader than /{$wgBlockCIDRLimit['IPv6']} are not accepted" + ), + array( 'code' => 'param_ip', 'info' => 'IP parameter is not valid' ), array( 'code' => 'param_user', 'info' => 'User parameter may not be empty' ), array( 'code' => 'param_user', 'info' => 'User name user is not valid' ), array( 'show' ), -- 2.20.1