From f523427459613e429a8851f6e66885b5cdef12ed Mon Sep 17 00:00:00 2001 From: Carsten Rose <carsten.rose@math.uzh.ch> Date: Sun, 6 May 2018 11:50:27 +0200 Subject: [PATCH] Client.php, Sanatize.php: Clean $_GET for 'type' and 'L'. They might be poisoned in cache. --- extension/qfq/qfq/helper/Sanitize.php | 32 +++++++++++++---- extension/qfq/qfq/store/Client.php | 6 +++- extension/qfq/tests/phpunit/SanitizeTest.php | 36 +++++++++++++++++++- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/extension/qfq/qfq/helper/Sanitize.php b/extension/qfq/qfq/helper/Sanitize.php index bb1b97adf..8f8a52988 100644 --- a/extension/qfq/qfq/helper/Sanitize.php +++ b/extension/qfq/qfq/helper/Sanitize.php @@ -54,12 +54,11 @@ class Sanitize { * @param string $value value to check * @param string $sanitizeClass * @param string $pattern Pattern as regexp - * @param array $decimalFormat with [ size, precision ] + * @param string $decimalFormat with 'size,precision' * @param string $mode SANITIZE_EXCEPTION | SANITIZE_EMPTY_STRING - * * @return string + * @throws CodeException * @throws UserFormException - * @throws \qfq\CodeException */ public static function sanitize($value, $sanitizeClass = SANITIZE_DEFAULT, $pattern = '', $decimalFormat = '', $mode = SANITIZE_EMPTY_STRING) { @@ -130,12 +129,11 @@ class Sanitize { * If check fails, depending on $mode, throws an UserException or return an empty string. * * @param string $value value to check - * @param $formElement + * @param $min + * @param $max * @param string $mode SANITIZE_EXCEPTION | SANITIZE_EMPTY_STRING - * * @return string * @throws UserFormException - * @throws \qfq\CodeException */ public static function checkMinMax($value, $min, $max, $mode = SANITIZE_EMPTY_STRING) { $errorCode = 0; @@ -223,7 +221,7 @@ class Sanitize { /** * Take the given $item (or iterates over all elements of the given array) and normalize the content. - * Only strings will be normalized. Sub arrays will be recursived normalized. Numeric content is skipped. + * Only strings will be normalized. Sub arrays will be normalized recursive. Numeric content is skipped. * Throws an exception for unknown content. * * It's important to normalize the user input: e.g. someone is searching for a record and input the search string @@ -279,4 +277,24 @@ class Sanitize { return $item; } + + /** + * Check a given $_GET[$key] is digit. + * If yes: do nothing + * If no: set to the first character (if it is a digit) else to an empty string. + * + * @param $key + */ + public static function digitCheckAndCleanGet($key) { + + if (isset($_GET[$key]) && !ctype_digit($_GET[$key])) { + if (ctype_digit($_GET[$key][0])) { + $_GET[$key] = $_GET[$key][0]; + } else { + $_GET[$key] = ''; + } + } + + } + } \ No newline at end of file diff --git a/extension/qfq/qfq/store/Client.php b/extension/qfq/qfq/store/Client.php index 5390b27ea..7070afebf 100644 --- a/extension/qfq/qfq/store/Client.php +++ b/extension/qfq/qfq/store/Client.php @@ -22,8 +22,12 @@ class Client { $cookie = array(); $server = array(); + // Dirty workaround to clean poisoned T3 cache + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_TYPE); + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + if (isset($_GET)) { - $get = \qfq\Sanitize::urlDecodeArr($_GET); + $get = Sanitize::urlDecodeArr($_GET); } if (isset($_POST)) { diff --git a/extension/qfq/tests/phpunit/SanitizeTest.php b/extension/qfq/tests/phpunit/SanitizeTest.php index a70e34c05..ebded83fc 100644 --- a/extension/qfq/tests/phpunit/SanitizeTest.php +++ b/extension/qfq/tests/phpunit/SanitizeTest.php @@ -119,7 +119,7 @@ class SanitizeTest extends \PHPUnit_Framework_TestCase { $this->assertEquals($val, Sanitize::checkMinMax($val, "", "56"), $msg); $this->assertEquals('', Sanitize::checkMinMax($val, "57", ""), $msg); - $this->assertEquals('', Sanitize::checkMinMax($val, "", "2" ), $msg); + $this->assertEquals('', Sanitize::checkMinMax($val, "", "2"), $msg); $this->assertEquals($val, Sanitize::checkMinMax($val, "0", "200"), $msg); $this->assertEquals($val, Sanitize::checkMinMax($val, "-100", "200"), $msg); @@ -337,4 +337,38 @@ class SanitizeTest extends \PHPUnit_Framework_TestCase { $this->assertEquals('test_./_oe', Sanitize::safeFilename($value, false, true)); } + + /** + * Test string, numeric + * + * @throws CodeException + */ + public function testDigitCheckAndCleanGet() { + + unset ($_GET[CLIENT_PAGE_LANGUAGE]); + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals(false, isset($_GET[CLIENT_PAGE_LANGUAGE])); + + $_GET[CLIENT_PAGE_LANGUAGE] = ''; + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals($_GET[CLIENT_PAGE_LANGUAGE], ''); + + $_GET[CLIENT_PAGE_LANGUAGE] = '0'; + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals($_GET[CLIENT_PAGE_LANGUAGE], '0'); + + $_GET[CLIENT_PAGE_LANGUAGE] = '1234'; + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals($_GET[CLIENT_PAGE_LANGUAGE], '1234'); + + $_GET[CLIENT_PAGE_LANGUAGE] = 'abc'; + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals($_GET[CLIENT_PAGE_LANGUAGE], ''); + + $_GET[CLIENT_PAGE_LANGUAGE] = '54abc'; + Sanitize::digitCheckAndCleanGet(CLIENT_PAGE_LANGUAGE); + $this->assertEquals($_GET[CLIENT_PAGE_LANGUAGE], '5'); + } + + } -- GitLab