diff --git a/core/Common.php b/core/Common.php index 64213d13188..c0605c3af39 100644 --- a/core/Common.php +++ b/core/Common.php @@ -10,6 +10,7 @@ use Exception; use Piwik\Container\StaticContainer; +use Piwik\Http\RequestSanitizer; use Piwik\Intl\Data\Provider\LanguageDataProvider; use Piwik\Intl\Data\Provider\RegionDataProvider; use Piwik\Plugins\UserCountry\LocationProvider\DefaultProvider; @@ -28,9 +29,6 @@ class Common const REFERRER_TYPE_WEBSITE = 3; const REFERRER_TYPE_CAMPAIGN = 6; - // Flag used with htmlspecialchar. See php.net/htmlspecialchars. - const HTML_ENCODING_QUOTE_STYLE = ENT_QUOTES; - public static $isCliMode = null; /* @@ -263,33 +261,9 @@ public static function mb_strtolower($string) */ public static function sanitizeInputValues($value, $alreadyStripslashed = false) { - if (is_numeric($value)) { - return $value; - } elseif (is_string($value)) { - $value = self::sanitizeString($value); - - if (!$alreadyStripslashed) { - // a JSON array was already stripslashed, don't do it again for each value - - $value = self::undoMagicQuotes($value); - } - } elseif (is_array($value)) { - foreach (array_keys($value) as $key) { - $newKey = $key; - $newKey = self::sanitizeInputValues($newKey, $alreadyStripslashed); - if ($key != $newKey) { - $value[$newKey] = $value[$key]; - unset($value[$key]); - } - - $value[$newKey] = self::sanitizeInputValues($value[$newKey], $alreadyStripslashed); - } - } elseif (!is_null($value) - && !is_bool($value) - ) { - throw new Exception("The value to escape has not a supported type. Value = " . var_export($value, true)); - } - return $value; + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + return $requestSanitizer->sanitizeValues($value, $alreadyStripslashed); } /** @@ -300,37 +274,9 @@ public static function sanitizeInputValues($value, $alreadyStripslashed = false) */ public static function sanitizeInputValue($value) { - $value = self::sanitizeLineBreaks($value); - $value = self::sanitizeString($value); - return $value; - } - - /** - * Sanitize a single input value - * - * @param $value - * @return string - */ - private static function sanitizeString($value) - { - // $_GET and $_REQUEST already urldecode()'d - // decode - // note: before php 5.2.7, htmlspecialchars() double encodes &#x hex items - $value = html_entity_decode($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); - - $value = self::sanitizeNullBytes($value); - - // escape - $tmp = @htmlspecialchars($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); - - // note: php 5.2.5 and above, htmlspecialchars is destructive if input is not UTF-8 - if ($value != '' && $tmp == '') { - // convert and escape - $value = utf8_encode($value); - $tmp = htmlspecialchars($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); - return $tmp; - } - return $tmp; + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + return $requestSanitizer->sanitizeValue($value); } /** @@ -341,7 +287,9 @@ private static function sanitizeString($value) */ public static function unsanitizeInputValue($value) { - return htmlspecialchars_decode($value, self::HTML_ENCODING_QUOTE_STYLE); + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + return $requestSanitizer->unsanitizeValue($value); } /** @@ -360,54 +308,9 @@ public static function unsanitizeInputValue($value) */ public static function unsanitizeInputValues($value) { - if (is_array($value)) { - $result = array(); - foreach ($value as $key => $arrayValue) { - $result[$key] = self::unsanitizeInputValues($arrayValue); - } - return $result; - } else { - return self::unsanitizeInputValue($value); - } - } - - /** - * Undo the damage caused by magic_quotes; deprecated in php 5.3 but not removed until php 5.4 - * - * @param string - * @return string modified or not - */ - private static function undoMagicQuotes($value) - { - static $shouldUndo; - - if (!isset($shouldUndo)) { - $shouldUndo = version_compare(PHP_VERSION, '5.4', '<') && get_magic_quotes_gpc(); - } - - if ($shouldUndo) { - $value = stripslashes($value); - } - - return $value; - } - - /** - * @param string $value - * @return string Line breaks and line carriage removed - */ - public static function sanitizeLineBreaks($value) - { - return str_replace(array("\n", "\r"), '', $value); - } - - /** - * @param string $value - * @return string Null bytes removed - */ - public static function sanitizeNullBytes($value) - { - return str_replace(array("\0"), '', $value); + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + return $requestSanitizer->unsanitizeValues($value); } /** @@ -468,9 +371,9 @@ public static function getRequestVar($varName, $varDefault = null, $varType = nu // we deal w/ json differently if ($varType == 'json') { - $value = self::undoMagicQuotes($requestArrayToUse[$varName]); - $value = json_decode($value, $assoc = true); - return self::sanitizeInputValues($value, $alreadyStripslashed = true); + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + return $requestSanitizer->sanitizeJsonValues($requestArrayToUse[$varName]); } $value = self::sanitizeInputValues($requestArrayToUse[$varName]); diff --git a/core/ExceptionHandler.php b/core/ExceptionHandler.php index ac61707a29b..e914438a48f 100644 --- a/core/ExceptionHandler.php +++ b/core/ExceptionHandler.php @@ -10,6 +10,7 @@ use Exception; use Piwik\Container\ContainerDoesNotExistException; +use Piwik\Http\RequestSanitizer; use Piwik\Plugins\CoreAdminHome\CustomLogo; /** @@ -68,7 +69,8 @@ private static function getErrorResponse(Exception $ex) $message = $ex->getMessage(); if (!method_exists($ex, 'isHtmlMessage') || !$ex->isHtmlMessage()) { - $message = Common::sanitizeInputValue($message); + $sanitizer = new RequestSanitizer(); + $message = $sanitizer->sanitizeValue($message); } $logo = new CustomLogo(); diff --git a/core/Http/RequestSanitizer.php b/core/Http/RequestSanitizer.php new file mode 100644 index 00000000000..ae19a2b55e2 --- /dev/null +++ b/core/Http/RequestSanitizer.php @@ -0,0 +1,189 @@ +sanitizeLineBreaks($value); + $value = $this->sanitizeString($value); + + if (!$alreadyStripslashed) { // a JSON array was already stripslashed, don't do it again for each value + $value = $this->undoMagicQuotes($value); + } + } else if (!is_null($value) + && !is_bool($value) + ) { + throw new \Exception("The value to escape has not a supported type. Value = " . var_export($value, true)); + } + return $value; + } + + /** + * Unsanitizes a single value. Replaces escaped HTML entities with the actual characters. + * + * @param string $value + * @return string + */ + public function unsanitizeValue($value) + { + return htmlspecialchars_decode($value, self::HTML_ENCODING_QUOTE_STYLE); + } + + /** + * Sanitize an array of values recursively. + * + * @param mixed $value If an array, it is sanitized recursively. Keys are sanitized as well as values. If not, + * it is sanitized normally. + * @param bool $alreadyStripslashed Implementation detail, ignore. + * @return mixed + */ + public function sanitizeValues($value, $alreadyStripslashed = false) + { + if (is_array($value)) { + foreach (array_keys($value) as $key) { + $newKey = $this->sanitizeValue($key, $alreadyStripslashed); + + if ($key != $newKey) { + $value[$newKey] = $value[$key]; + unset($value[$key]); + } + + $value[$newKey] = $this->sanitizeValues($value[$newKey], $alreadyStripslashed); + } + return $value; + } else { + return $this->sanitizeValue($value); + } + } + + /** + * Unsanitize array values recursively. + * + * @param mixed $values If an array, it is unsanitized recursively. Keys are not unsanitized. If not an array, + * it is sanitized normally. + * @return mixed + */ + public function unsanitizeValues($value) + { + if (is_array($value)) { + $result = array(); + foreach ($value as $key => $arrayValue) { + $result[$key] = $this->unsanitizeValues($arrayValue); + } + return $result; + } else { + return $this->unsanitizeValue($value); + } + } + + /** + * @param string $value + * @return string Line breaks and line carriage removed + */ + public function sanitizeLineBreaks($value) + { + return str_replace(array("\n", "\r"), '', $value); + } + + /** + * Sanitizes a JSON string by value. + * + * @param string $value Parses a JSON string into an array and then sanitizes every element recursively. Array + * keys are sanitized as well. + * @return array + */ + public function sanitizeJsonValues($value) + { + $value = $this->undoMagicQuotes($value); + $value = json_decode($value, $assoc = true); + return $this->sanitizeValues($value, $alreadyStripslashed = true); + } + + /** + * Sanitize a single input value + * + * @param $value + * @return string + */ + private function sanitizeString($value) + { + // $_GET and $_REQUEST already urldecode()'d + // decode + // note: before php 5.2.7, htmlspecialchars() double encodes &#x hex items + $value = html_entity_decode($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); + + $value = $this->sanitizeNullBytes($value); + + // escape + $tmp = @htmlspecialchars($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); + + // note: php 5.2.5 and above, htmlspecialchars is destructive if input is not UTF-8 + if ($value != '' && $tmp == '') { + // convert and escape + $value = utf8_encode($value); + $tmp = htmlspecialchars($value, self::HTML_ENCODING_QUOTE_STYLE, 'UTF-8'); + return $tmp; + } + return $tmp; + } + + /** + * @param string $value + * @return string Null bytes removed + */ + private function sanitizeNullBytes($value) + { + return str_replace(array("\0"), '', $value); + } + + /** + * Undo the damage caused by magic_quotes; deprecated in php 5.3 but not removed until php 5.4 + * + * @param string + * @return string modified or not + */ + private function undoMagicQuotes($value) + { + static $shouldUndo; + + if (!isset($shouldUndo)) { + $shouldUndo = version_compare(PHP_VERSION, '5.4', '<') && get_magic_quotes_gpc(); + } + + if ($shouldUndo) { + $value = stripslashes($value); + } + + return $value; + } +} \ No newline at end of file diff --git a/plugins/BulkTracking/Tracker/Requests.php b/plugins/BulkTracking/Tracker/Requests.php index af4eb23a158..df0d3a55fae 100644 --- a/plugins/BulkTracking/Tracker/Requests.php +++ b/plugins/BulkTracking/Tracker/Requests.php @@ -10,6 +10,8 @@ use Exception; use Piwik\Common; +use Piwik\Container\StaticContainer; +use Piwik\Http\RequestSanitizer; use Piwik\Tracker\Request; use Piwik\Tracker\TrackerConfig; @@ -67,7 +69,10 @@ public function isUsingBulkRequest($rawData) public function getRequestsArrayFromBulkRequest($rawData) { $rawData = trim($rawData); - $rawData = Common::sanitizeLineBreaks($rawData); + + /** @var RequestSanitizer $requestSanitizer */ + $requestSanitizer = StaticContainer::get('Piwik\Http\RequestSanitizer'); + $rawData = $requestSanitizer->sanitizeLineBreaks($rawData); // POST data can be array of string URLs or array of arrays w/ visit info $jsonData = json_decode($rawData, $assoc = true); diff --git a/tests/PHPUnit/Unit/CommonTest.php b/tests/PHPUnit/Unit/CommonTest.php index 8aa85ed550a..611c4e0128d 100644 --- a/tests/PHPUnit/Unit/CommonTest.php +++ b/tests/PHPUnit/Unit/CommonTest.php @@ -215,7 +215,7 @@ public function getRequestVarValues() array(array("test", 1345524, array("gaga")), array(), 'array', array("test", 1345524, array("gaga"))), // array as a default value / types array(array("test", 1345524, array("gaga")), 45, 'string', "45"), array(array("test", 1345524, array("gaga")), array(1), 'array', array("test", 1345524, array("gaga"))), - array(array("test", 1345524, "Start of hello\nworld\n\t", array("gaga")), array(1), 'array', array("test", 1345524, "Start of hello\nworld\n\t", array("gaga"))), + array(array("test", 1345524, "Start of hello\nworld\n\t", array("gaga")), array(1), 'array', array("test", 1345524, "Start of helloworld\t", array("gaga"))), array(array("test", 1345524, array("gaga")), 4, 'int', 4), array('', array(1), 'array', array(1)), array('', array(), 'array', array()), diff --git a/tests/resources/staticFileServer.php b/tests/resources/staticFileServer.php index 696acebdcfd..724dccfaa12 100644 --- a/tests/resources/staticFileServer.php +++ b/tests/resources/staticFileServer.php @@ -41,6 +41,9 @@ define("PARTIAL_BYTE_START", 1204); define("PARTIAL_BYTE_END", 14724); +$environment = new \Piwik\Application\Environment(null); +$environment->init(); + /** * If the static file server has been requested, the response sent back to the browser will be the content produced by * the execution of Piwik:serverStaticFile(). In this case, unit tests won't be executed @@ -56,9 +59,6 @@ SRV_MODE_REQUEST_VAR . " must be provided."); } -$environment = new \Piwik\Application\Environment(null); -$environment->init(); - switch ($staticFileServerMode) { // The static file server calls Piwik::serverStaticFile with a null file case NULL_FILE_SRV_MODE: