From 65a626f94009f4c83f8290ae1e64e0aae7e02131 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sat, 23 Mar 2024 11:27:25 +0100 Subject: [PATCH] inputs sanitation & remove some obsolete version checks using filter_vars instead of filter_input, because our unit tests depend on manipulating global arrays, which are not used by filter_input - we would have to mock the function in the unit testing, it therefore is cleaner to use the same code paths in testing as in production some inputs in I18n and TrafficLimiter remain unfiltered, since we already validate them by other means (IP lib and/or preg_match) our minimum PHP version is 7.3, so we can drop the two < 5.6 fallback checks --- CHANGELOG.md | 2 ++ lib/Model/AbstractModel.php | 2 +- lib/Request.php | 39 ++++++++++++++++++------------------- tst/Bootstrap.php | 2 +- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6c938ca..b431b5b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ ## 1.7.2 (not yet released) * ADDED: Allow use of `shortenviayourls` in query parameters (#1267) +* ADDED: Input sanitation to some not yet filtered query and server parameters * CHANGED: "Send" button now labeled "Create" (#946) +* CHANGED: drop some PHP < 5.6 fallbacks, minimum version is PHP 7.3 as of release 1.6.0 * FIXED: Add cache control headers also to API calls (#1263) * FIXED: Shortened paste URL does not appear in email (#606) diff --git a/lib/Model/AbstractModel.php b/lib/Model/AbstractModel.php index 2e77cef9..de7b9a02 100644 --- a/lib/Model/AbstractModel.php +++ b/lib/Model/AbstractModel.php @@ -108,7 +108,7 @@ abstract class AbstractModel $this->_data = $data; // calculate a 64 bit checksum to avoid collisions - $this->setId(hash(version_compare(PHP_VERSION, '5.6', '<') ? 'fnv164' : 'fnv1a64', $data['ct'])); + $this->setId(hash('fnv1a64', $data['ct'])); } /** diff --git a/lib/Request.php b/lib/Request.php index ef6c7043..94202744 100644 --- a/lib/Request.php +++ b/lib/Request.php @@ -82,13 +82,10 @@ class Request */ private function getPasteId() { - // RegEx to check for valid paste ID (16 base64 chars) - $pasteIdRegEx = '/^[a-f0-9]{16}$/'; - foreach ($_GET as $key => $value) { - // only return if value is empty and key matches RegEx - if (($value === '') and preg_match($pasteIdRegEx, $key, $match)) { - return $match[0]; + // only return if value is empty and key is 16 hex chars + if (($value === '') && strlen($key) === 16 && ctype_xdigit($key)) { + return $key; } } @@ -121,7 +118,13 @@ class Request } break; default: - $this->_params = $_GET; + $this->_params = filter_var_array($_GET, array( + 'deletetoken' => FILTER_SANITIZE_SPECIAL_CHARS, + 'jsonld' => FILTER_SANITIZE_SPECIAL_CHARS, + 'link' => FILTER_SANITIZE_URL, + 'pasteid' => FILTER_SANITIZE_SPECIAL_CHARS, + 'shortenviayourls' => FILTER_SANITIZE_SPECIAL_CHARS, + ), false); } if ( !array_key_exists('pasteid', $this->_params) && @@ -209,9 +212,8 @@ class Request */ public function getHost() { - return array_key_exists('HTTP_HOST', $_SERVER) ? - htmlspecialchars($_SERVER['HTTP_HOST']) : - 'localhost'; + $host = array_key_exists('HTTP_HOST', $_SERVER) ? filter_var($_SERVER['HTTP_HOST'], FILTER_SANITIZE_URL) : ''; + return empty($host) ? 'localhost' : $host; } /** @@ -222,10 +224,8 @@ class Request */ public function getRequestUri() { - return array_key_exists('REQUEST_URI', $_SERVER) ? - htmlspecialchars( - parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) - ) : '/'; + $uri = array_key_exists('REQUEST_URI', $_SERVER) ? filter_var($_SERVER['REQUEST_URI'], FILTER_SANITIZE_URL) : ''; + return empty($uri) ? '/' : $uri; } /** @@ -275,10 +275,9 @@ class Request } // advanced case: media type negotiation - $mediaTypes = array(); if ($hasAcceptHeader) { - $mediaTypeRanges = explode(',', trim($acceptHeader)); - foreach ($mediaTypeRanges as $mediaTypeRange) { + $mediaTypes = array(); + foreach (explode(',', trim($acceptHeader)) as $mediaTypeRange) { if (preg_match( '#(\*/\*|[a-z\-]+/[a-z\-+*]+(?:\s*;\s*[^q]\S*)*)(?:\s*;\s*q\s*=\s*(0(?:\.\d{0,3})|1(?:\.0{0,3})))?#', trim($mediaTypeRange), $match @@ -287,6 +286,9 @@ class Request $match[2] = '1.0'; } else { $match[2] = (string) floatval($match[2]); + if ($match[2] === '0.0') { + continue; + } } if (!isset($mediaTypes[$match[2]])) { $mediaTypes[$match[2]] = array(); @@ -296,9 +298,6 @@ class Request } krsort($mediaTypes); foreach ($mediaTypes as $acceptedQuality => $acceptedValues) { - if ($acceptedQuality === '0.0') { - continue; - } foreach ($acceptedValues as $acceptedValue) { if ( strpos($acceptedValue, self::MIME_HTML) === 0 || diff --git a/tst/Bootstrap.php b/tst/Bootstrap.php index 3af4db0a..7f4caa32 100644 --- a/tst/Bootstrap.php +++ b/tst/Bootstrap.php @@ -696,7 +696,7 @@ class Helper */ public static function getPasteId() { - return version_compare(PHP_VERSION, '5.6', '<') ? hash('fnv164', self::$pasteV2['ct']) : self::$pasteid; + return self::$pasteid; } /**