From: Bartosz Dziewoński <matma.rex@gmail.com>
Date: Fri, 30 Aug 2019 17:21:34 +0000 (+0200)
Subject: ApiBase: Always validate that 'limit' is numeric
X-Git-Tag: 1.34.0-rc.0~403^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=a9199bf8548a228584f69ca32724ee296c931d92;p=lhc%2Fweb%2Fwiklou.git

ApiBase: Always validate that 'limit' is numeric

Bug: T231582
Change-Id: I956d4d623bfeace1b542039283e04a970fd40121
---

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 8b6a3e582f..9680601a95 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1308,8 +1308,15 @@ abstract class ApiBase extends ContextSource {
 						}
 						break;
 					case 'limit':
+						// Must be a number or 'max'
+						if ( $value !== 'max' ) {
+							$value = (int)$value;
+						}
+						if ( $multi ) {
+							self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
+						}
 						if ( !$parseLimit ) {
-							// Don't do any validation whatsoever
+							// Don't do min/max validation and don't parse 'max'
 							break;
 						}
 						if ( !isset( $paramSettings[self::PARAM_MAX] )
@@ -1320,21 +1327,16 @@ abstract class ApiBase extends ContextSource {
 								"MAX1 or MAX2 are not defined for the limit $encParamName"
 							);
 						}
-						if ( $multi ) {
-							self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
-						}
-						$min = $paramSettings[self::PARAM_MIN] ?? 0;
-						if ( $value == 'max' ) {
+						if ( $value === 'max' ) {
 							$value = $this->getMain()->canApiHighLimits()
 								? $paramSettings[self::PARAM_MAX2]
 								: $paramSettings[self::PARAM_MAX];
 							$this->getResult()->addParsedLimit( $this->getModuleName(), $value );
 						} else {
-							$value = (int)$value;
 							$this->validateLimit(
 								$paramName,
 								$value,
-								$min,
+								$paramSettings[self::PARAM_MIN] ?? 0,
 								$paramSettings[self::PARAM_MAX],
 								$paramSettings[self::PARAM_MAX2]
 							);
diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php
index 6a44ff3594..c49d669595 100644
--- a/tests/phpunit/includes/api/ApiBaseTest.php
+++ b/tests/phpunit/includes/api/ApiBaseTest.php
@@ -889,10 +889,24 @@ class ApiBaseTest extends ApiTestCase {
 				[],
 				[ 'internalmode' => false ],
 			],
-			'Limit with parseLimits false' => [
+			'Limit with parseLimits false (numeric)' => [
 				'100',
 				[ ApiBase::PARAM_TYPE => 'limit' ],
-				'100',
+				100,
+				[],
+				[ 'parseLimits' => false ],
+			],
+			'Limit with parseLimits false (max)' => [
+				'max',
+				[ ApiBase::PARAM_TYPE => 'limit' ],
+				'max',
+				[],
+				[ 'parseLimits' => false ],
+			],
+			'Limit with parseLimits false (invalid)' => [
+				'kitten',
+				[ ApiBase::PARAM_TYPE => 'limit' ],
+				0,
 				[],
 				[ 'parseLimits' => false ],
 			],
@@ -901,7 +915,6 @@ class ApiBaseTest extends ApiTestCase {
 				[
 					ApiBase::PARAM_TYPE => 'limit',
 					ApiBase::PARAM_MAX2 => 10,
-					ApiBase::PARAM_ISMULTI => true,
 				],
 				new MWException(
 					'Internal error in ApiBase::getParameterFromSettings: ' .
@@ -913,7 +926,6 @@ class ApiBaseTest extends ApiTestCase {
 				[
 					ApiBase::PARAM_TYPE => 'limit',
 					ApiBase::PARAM_MAX => 10,
-					ApiBase::PARAM_ISMULTI => true,
 				],
 				new MWException(
 					'Internal error in ApiBase::getParameterFromSettings: ' .