Skip to content

GitLab

Unverified Commit c1959643 authored by Cédric Anne's avatar Cédric Anne Committed by GitHub
Browse files

Move sanitize logic into a dedicated class

parent 94710084
Hide whitespace changes
Inline Side-by-side
inc/queuednotification.class.php
View file @ c1959643
......@@ -34,6 +34,7 @@ if (!defined('GLPI_ROOT')) {
die("Sorry. You can't access this file directly");
}
use Glpi\Toolbox\Sanitizer;
/** QueuedNotification class
*
......@@ -436,7 +437,7 @@ class QueuedNotification extends CommonDBTM {
*/
public function sendById($ID) {
if ($this->getFromDB($ID)) {
$this->fields = Toolbox::unclean_cross_side_scripting_deep($this->fields);
$this->fields = Sanitizer::unsanitize($this->fields);
$mode = $this->getField('mode');
$eventclass = 'NotificationEvent' . ucfirst($mode);
......@@ -558,7 +559,7 @@ class QueuedNotification extends CommonDBTM {
);
foreach ($pendings as $mode => $data) {
$data = Toolbox::unclean_cross_side_scripting_deep($data);
$data = Sanitizer::unsanitize($data);
$eventclass = 'NotificationEvent' . ucfirst($mode);
$conf = Notification_NotificationTemplate::getMode($mode);
......@@ -631,7 +632,7 @@ class QueuedNotification extends CommonDBTM {
);
foreach ($pendings as $mode => $data) {
$data = Toolbox::unclean_cross_side_scripting_deep($data);
$data = Sanitizer::unsanitize($data);
$eventclass = Notification_NotificationTemplate::getModeClass($mode, 'event');
$eventclass::send($data);
......@@ -745,7 +746,7 @@ class QueuedNotification extends CommonDBTM {
echo "<tr class='tab_bg_1 top' >";
echo "<td colspan='2' class='queuemail_preview'>";
echo self::cleanHtml(Toolbox::unclean_cross_side_scripting_deep($this->fields['body_html']));
echo self::cleanHtml(Sanitizer::unsanitize($this->fields['body_html']));
echo "</td>";
echo "<td colspan='2'>".nl2br($this->fields['body_text'], false)."</td>";
echo "</tr>";
......
inc/rulecriteria.class.php
View file @ c1959643
......@@ -34,6 +34,8 @@ if (!defined('GLPI_ROOT')) {
die("Sorry. You can't access this file directly");
}
use Glpi\Toolbox\Sanitizer;
/// Criteria Rule class
class RuleCriteria extends CommonDBChild {
......@@ -443,7 +445,7 @@ class RuleCriteria extends CommonDBChild {
case Rule::REGEX_MATCH :
$results = [];
// Permit use < and >
$pattern = Toolbox::unclean_cross_side_scripting_deep($pattern);
$pattern = Sanitizer::unsanitize($pattern);
if (preg_match_all($pattern."i", $field, $results)>0) {
// Drop $result[0] : complete match result
array_shift($results);
......@@ -462,7 +464,7 @@ class RuleCriteria extends CommonDBChild {
case Rule::REGEX_NOT_MATCH :
// Permit use < and >
$pattern = Toolbox::unclean_cross_side_scripting_deep($pattern);
$pattern = Sanitizer::unsanitize($pattern);
if (preg_match($pattern."i", $field) == 0) {
$criterias_results[$criteria] = $pattern;
return true;
......
inc/ruleright.class.php
View file @ c1959643
......@@ -34,6 +34,8 @@ if (!defined('GLPI_ROOT')) {
die("Sorry. You can't access this file directly");
}
use Glpi\Toolbox\Sanitizer;
/**
* RuleRight Class
*
......@@ -169,7 +171,7 @@ class RuleRight extends Rule {
break;
case "_affect_entity_by_completename" :
$res = Toolbox::unclean_cross_side_scripting_deep($res);
$res = Sanitizer::unsanitize($res);
$entity_found = Entity::getEntityIDByCompletename(addslashes($res));
break;
......
inc/ruleticket.class.php
View file @ c1959643
......@@ -34,6 +34,7 @@ if (!defined('GLPI_ROOT')) {
die("Sorry. You can't access this file directly");
}
use Glpi\Toolbox\Sanitizer;
class RuleTicket extends Rule {
......@@ -238,9 +239,7 @@ class RuleTicket extends Rule {
$solution_content = $template->getRenderedContent($parent);
// Sanitize generated HTML before adding it in DB
$solution_content = Toolbox::clean_cross_side_scripting_deep(
Toolbox::addslashes_deep($solution_content)
);
$solution_content = Sanitizer::sanitize($solution_content, true);
$solution = new ITILSolution();
$solution->add([
......
inc/search.class.php
View file @ c1959643
......@@ -36,6 +36,7 @@ if (!defined('GLPI_ROOT')) {
use Glpi\Toolbox\DataExport;
use Glpi\Toolbox\RichText;
use Glpi\Toolbox\Sanitizer;
/**
* Search Class
......@@ -7852,7 +7853,7 @@ JAVASCRIPT;
**/
static function makeTextSearchValue($val) {
// Unclean to permit < and > search
$val = Toolbox::unclean_cross_side_scripting_deep($val);
$val = Sanitizer::unsanitize($val);
// escape _ char used as wildcard in mysql likes
$val = str_replace('_', '\\_', $val);
......
inc/toolbox.class.php
View file @ c1959643
......@@ -34,6 +34,7 @@ use Glpi\Console\Application;
use Glpi\Event;
use Glpi\Mail\Protocol\ProtocolInterface;
use Glpi\System\RequirementsManager;
use Glpi\Toolbox\Sanitizer;
use Laminas\Mail\Storage\AbstractStorage;
use Mexitek\PHPColors\Color;
use Monolog\Logger;
......@@ -356,19 +357,12 @@ class Toolbox {
* @return array|string clean item
*
* @see unclean_cross_side_scripting_deep*
*
* @deprecated 10.0.0
**/
static function clean_cross_side_scripting_deep($value) {
if ((array) $value === $value) {
return array_map([__CLASS__, 'clean_cross_side_scripting_deep'], $value);
}
if (!is_string($value)) {
return $value;
}
$mapping = self::getXssCleanCharsMapping();
return str_replace(array_keys($mapping), array_values($mapping), $value);
Toolbox::deprecated('Use "Glpi\Toolbox\Sanitizer::sanitize()"');
return Sanitizer::sanitize($value);
}
......@@ -380,47 +374,12 @@ class Toolbox {
* @return array|string unclean item
*
* @see clean_cross_side_scripting_deep()
*
* @deprecated 10.0.0
**/
static function unclean_cross_side_scripting_deep($value) {
if ((array) $value === $value) {
return array_map([__CLASS__, 'unclean_cross_side_scripting_deep'], $value);
}
if (!is_string($value)) {
return $value;
}
$mapping = self::getXssCleanCharsMapping();
foreach ($mapping as $htmlentity) {
if (strpos($value, $htmlentity) !== false) {
// Value was cleaned using new char mapping, so it must be uncleaned with same mapping
$mapping = array_reverse(self::getXssCleanCharsMapping());
return str_replace(array_values($mapping), array_keys($mapping), $value);
}
}
// Fallback to old chars mapping
$in = ['<', '>'];
$out = ['&lt;', '&gt;'];
return str_replace($out, $in, $value);
}
/**
* Get chars mapping used for XSS cleaning process.
* Keys are chars and values are corresponding html entities.
*
* @return string[]
*/
private static function getXssCleanCharsMapping() {
// Order is important here.
// `str_replace` acts as a loop, so `&` replacement must be at first place, as other replacements
// will produce new `&` that should not be replaced.
return [
'&' => '&#38;',
'<' => '&#60;',
'>' => '&#62;',
];
Toolbox::deprecated('Use "Glpi\Toolbox\Sanitizer::unsanitize()"');
return Sanitizer::unsanitize($value);
}
......@@ -2589,11 +2548,12 @@ class Toolbox {
* @param array $array
*
* @return array
*
* @deprecated 10.0.0
*/
static public function sanitize($array) {
$array = array_map('Toolbox::addslashes_deep', $array);
$array = array_map('Toolbox::clean_cross_side_scripting_deep', $array);
return $array;
Toolbox::deprecated('Use "Glpi\Toolbox\Sanitizer::unsanitize($value, true)"');
return Sanitizer::sanitize($array, true);
}
/**
......@@ -2691,11 +2651,11 @@ class Toolbox {
// 1 - Replace direct tag (with prefix and suffix) by the image
$content_text = preg_replace('/'.Document::getImageTag($image['tag']).'/',
self::clean_cross_side_scripting_deep($img), $content_text);
Sanitizer::sanitize($img), $content_text);
// 2 - Replace img with tag in id attribute by the image
$regex = '/<img[^>]+' . preg_quote($image['tag'], '/') . '[^<]+>/im';
preg_match_all($regex, self::unclean_cross_side_scripting_deep($content_text), $matches);
preg_match_all($regex, Sanitizer::unsanitize($content_text), $matches);
foreach ($matches[0] as $match_img) {
//retrieve dimensions
$width = $height = null;
......@@ -2729,9 +2689,9 @@ class Toolbox {
$content_text = str_replace(
$match_img,
$new_image,
self::unclean_cross_side_scripting_deep($content_text)
Sanitizer::unsanitize($content_text)
);
$content_text = self::clean_cross_side_scripting_deep($content_text);
$content_text = Sanitizer::sanitize($content_text);
}
// If the tag is from another ticket : link document to ticket
......@@ -2796,13 +2756,13 @@ class Toolbox {
* @return string html content
**/
static function cleanTagOrImage($content, array $tags) {
$content = Toolbox::unclean_cross_side_scripting_deep($content);
$content = Sanitizer::unsanitize($content);
foreach ($tags as $tag) {
$content = preg_replace("/<img.*alt=['|\"]".$tag."['|\"][^>]*\>/", "<p></p>", $content);
}
$content = Toolbox::clean_cross_side_scripting_deep($content);
$content = Sanitizer::sanitize($content);
return $content;
}
......@@ -2875,7 +2835,7 @@ class Toolbox {
*/
public static function getRemoteIpAddress() {
return (isset($_SERVER["HTTP_X_FORWARDED_FOR"]) ?
self::clean_cross_side_scripting_deep($_SERVER["HTTP_X_FORWARDED_FOR"]):
Sanitizer::sanitize($_SERVER["HTTP_X_FORWARDED_FOR"]):
$_SERVER["REMOTE_ADDR"]);
}
......@@ -3065,7 +3025,7 @@ class Toolbox {
public static function stripTags(string $str, bool $sanitized_input = false): string {
if ($sanitized_input) {
$str = self::unclean_cross_side_scripting_deep($str);
$str = Sanitizer::unsanitize($str);
}
return strip_tags($str);
......
inc/toolbox/dataexport.class.php
View file @ c1959643
......@@ -32,8 +32,6 @@
namespace Glpi\Toolbox;
use Toolbox;
class DataExport {
/**
......@@ -45,7 +43,7 @@ class DataExport {
* @return string
*/
public static function normalizeValueForTextExport(string $value): string {
$value = Toolbox::unclean_cross_side_scripting_deep($value);
$value = Sanitizer::unsanitize($value);
if (RichText::isRichTextHtmlContent($value)) {
// Remove invisible contents (tooltips for instance)
......
inc/toolbox/richtext.class.php
View file @ c1959643
......@@ -56,7 +56,7 @@ class RichText {
}
if ($sanitized_input) {
$content = Toolbox::unclean_cross_side_scripting_deep($content);
$content = Sanitizer::unsanitize($content);
}
$content = self::normalizeHtmlContent($content, true);
......@@ -109,7 +109,7 @@ class RichText {
global $CFG_GLPI;
if ($sanitized_input) {
$content = Toolbox::unclean_cross_side_scripting_deep($content);
$content = Sanitizer::unsanitize($content);
}
$content = self::normalizeHtmlContent($content, false);
......
inc/toolbox/sanitizer.class.php 0 → 100644
View file @ c1959643
<?php
/**
* ---------------------------------------------------------------------
* GLPI - Gestionnaire Libre de Parc Informatique
* Copyright (C) 2015-2021 Teclib' and contributors.
*
* http://glpi-project.org
*
* based on GLPI - Gestionnaire Libre de Parc Informatique
* Copyright (C) 2003-2014 by the INDEPNET Development Team.
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* GLPI is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* GLPI is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with GLPI. If not, see <http://www.gnu.org/licenses/>.
* ---------------------------------------------------------------------
*/
namespace Glpi\Toolbox;
use Toolbox;
class Sanitizer {
private const CHARS_MAPPING = [
'&' => '&#38;',
'<' => '&#60;',
'>' => '&#62;',
];
private const LEGACY_CHARS_MAPPING = [
'<' => '&lt;',
'>' => '&gt;',
];
/**
* Sanitize a value. Resulting value will have its HTML special chars converted into entities
* and would be printable in a HTML document without having to be escaped.
*
* @param mixed $value
* @param bool $db_escape
*
* @return mixed
*/
public static function sanitize($value, bool $db_escape = false) {
if (is_array($value)) {
return array_map(
function ($val) use ($db_escape) {
return self::sanitize($val, $db_escape);
},
$value
);
}
if (!is_string($value)) {
return $value;
}
if ($db_escape) {
$value = self::dbEscape($value);
}
$mapping = self::CHARS_MAPPING;
return str_replace(array_keys($mapping), array_values($mapping), $value);
}
/**
* Unsanitize a value. Reverts self::sanitize() transformation.
*
* @param mixed $value
* @param bool $db_unescape
*
* @return mixed
*/
public static function unsanitize($value, bool $db_unescape = false) {
if (is_array($value)) {
return array_map(
function ($val) use ($db_unescape) {
return self::unsanitize($val, $db_unescape);
},
$value
);
}
if (!is_string($value)) {
return $value;
}
$mapping = null;
foreach (self::CHARS_MAPPING as $htmlentity) {
if (strpos($value, $htmlentity) !== false) {
// Value was cleaned using new char mapping, so it must be uncleaned with same mapping
$mapping = self::CHARS_MAPPING;
break;
}
}
if ($mapping === null) {
$mapping = self::LEGACY_CHARS_MAPPING; // Fallback to legacy chars mapping
}
$mapping = array_reverse($mapping);
$value = str_replace(array_values($mapping), array_keys($mapping), $value);
if ($db_unescape) {
$value = self::dbUnescape($value);
}
return $value;
}
/**
* Check if value is sanitized.
*
* @param string $value
*
* @return bool
*/
public static function isSanitized(string $value): bool {
$special_chars_pattern = '/(<|>|(&(?!#?[a-z0-9]+;)))/i';
$sanitized_chars_pattern = '/(' . implode('|', array_merge(self::CHARS_MAPPING, self::LEGACY_CHARS_MAPPING)) . ')/';
return preg_match($special_chars_pattern, $value) === 0
&& preg_match($sanitized_chars_pattern, $value) === 1;
}
/**
* Escape special chars to protect DB queries.
*
* @param string $value
*
* @return string
*/
private static function dbEscape(string $value): string {
// TODO Toolbox::addslashes_deep() should be moved in current class,
// but it is widely used, so it will be done later.
return Toolbox::addslashes_deep($value);
}
/**
* Revert `mysqli::real_escape_string()` transformation.
* Inspired by https://stackoverflow.com/a/38769977
*
* @param string $value
*
* @return string
*/
private static function dbUnescape(string $value): string {
// stripslashes cannot be used here as it would produce "r" and "n" instead of "\r" and \n".
$search = ['x00', 'n', 'r', '\\', '\'', '"','x1a'];
$replace = ["\x00", "\n", "\r", "\\", "'", "\"", "\x1a"];
for ($i = 0; $i < strlen($value); $i++) {
if (substr($value, $i, 1) == '\\') {
foreach ($search as $index => $char) {
if ($i <= strlen($value) - strlen($char) && substr($value, $i + 1, strlen($char)) == $char) {
$value = substr_replace($value, $replace[$index], $i, strlen($char) + 1);
break;
}
}
}
}
return $value;
}
}
inc/user.class.php
View file @ c1959643
......@@ -35,6 +35,7 @@ if (!defined('GLPI_ROOT')) {
}
use Glpi\Exception\ForgetPasswordException;
use Glpi\Toolbox\Sanitizer;
use Sabre\VObject;
class User extends CommonDBTM {
......@@ -605,7 +606,7 @@ class User extends CommonDBTM {
if ($input["password"] == $input["password2"]) {
if (Config::validatePassword($input["password"])) {
$input["password"]
= Auth::getPasswordHash(Toolbox::unclean_cross_side_scripting_deep(stripslashes($input["password"])));
= Auth::getPasswordHash(Sanitizer::unsanitize(stripslashes($input["password"])));
$input['password_last_update'] = $_SESSION['glpi_currenttime'];
} else {
......@@ -801,7 +802,7 @@ class User extends CommonDBTM {
-strtotime($this->fields['password_forget_token_date'])) < DAY_TIMESTAMP)
&& $this->isEmail($input['email'])))) {
$input["password"]
= Auth::getPasswordHash(Toolbox::unclean_cross_side_scripting_deep(stripslashes($input["password"])));
= Auth::getPasswordHash(Sanitizer::unsanitize($input["password"], true));
$input['password_last_update'] = $_SESSION["glpi_currenttime"];
} else {
......@@ -1810,7 +1811,7 @@ class User extends CommonDBTM {
}
//Perform the search
$filter = Toolbox::unclean_cross_side_scripting_deep($filter);
$filter = Sanitizer::unsanitize($filter);
$sr = ldap_search($ds, $ldap_base_dn, $filter, $attrs);
//Get the result of the search as an array
......@@ -5368,7 +5369,7 @@ JAVASCRIPT;
*/
private static function getLdapFieldValue($map, array $res) {
$map = Toolbox::unclean_cross_side_scripting_deep($map);
$map = Sanitizer::unsanitize($map);
$ret = preg_replace_callback('/%{(.*)}/U',
function ($matches) use ($res) {
return (isset($res[0][$matches[1]][0]) ? $res[0][$matches[1]][0] : '');
......
tests/DbTestCase.php
View file @ c1959643
......@@ -30,6 +30,8 @@
* ---------------------------------------------------------------------
*/
use Glpi\Toolbox\Sanitizer;
// Generic test classe, to be extended for CommonDBTM Object
class DbTestCase extends \GLPITestCase {
......@@ -175,7 +177,7 @@ class DbTestCase extends \GLPITestCase {
*/
protected function createItem($itemtype, $input): CommonDBTM {
$item = new $itemtype();
$input = Toolbox::sanitize($input);
$input = Sanitizer::sanitize($input, true);
$id = $item->add($input);
$this->integer($id)->isGreaterThan(0);
......@@ -198,7 +200,7 @@ class DbTestCase extends \GLPITestCase {
protected function updateItem($itemtype, $id, $input) {
$item = new $itemtype();
$input['id'] = $id;
$input = Toolbox::sanitize($input);
$input = Sanitizer::sanitize($input, true);
$success = $item->update($input);
$this->boolean($success)->isTrue();
......
tests/functionnal/Dropdown.php
View file @ c1959643
......@@ -33,6 +33,7 @@
namespace tests\units;
use DbTestCase;
use Glpi\Toolbox\Sanitizer;
/* Test for inc/dropdown.class.php */
......@@ -122,7 +123,7 @@ class Dropdown extends DbTestCase {
public function testGetDropdownName() {
global $CFG_GLPI;
$encoded_sep = \Toolbox::clean_cross_side_scripting_deep(' > ');
$encoded_sep = Sanitizer::sanitize(' > ');
$ret = \Dropdown::getDropdownName('not_a_known_table', 1);
$this->string($ret)->isIdenticalTo('&nbsp;');
......@@ -795,7 +796,7 @@ class Dropdown extends DbTestCase {
}
protected function getDropdownConnectProvider() {
$encoded_sep = \Toolbox::clean_cross_side_scripting_deep('>');
$encoded_sep = Sanitizer::sanitize('>');
return [
[
......
tests/functionnal/NotificationTargetTicket.php
View file @ c1959643
......@@ -33,6 +33,7 @@
namespace tests\units;
use DbTestCase;
use Glpi\Toolbox\Sanitizer;
/* Test for inc/notificationtargetticket.class.php */
......@@ -79,7 +80,7 @@ class NotificationTargetTicket extends DbTestCase {
// advanced test for ##task.categorycomment## and ##task.categoryid## tags
// test of the getDataForObject for default language en_GB
$taskcat = getItemByTypeName('TaskCategory', '_subcat_1');
$encoded_sep = \Toolbox::clean_cross_side_scripting_deep('>');
$encoded_sep = Sanitizer::sanitize('>');
$expected = [
[
'##task.id##' => 1,
......
tests/functionnal/RuleTicket.php
View file @ c1959643
......@@ -36,6 +36,7 @@ use CommonITILValidation;
use Contract;
use ContractType;
use DbTestCase;
use Glpi\Toolbox\Sanitizer;
use Group_User;
use ITILFollowup;
use ITILFollowupTemplate;
......@@ -482,7 +483,7 @@ class RuleTicket extends DbTestCase {
$this->boolean($itilSolution->getFromDBByCrit([
'items_id' => $tickets_id,
'itemtype' => 'Ticket',