From d33b612186671c84ff157675a176a55b754aac09 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 25 Nov 2020 21:02:27 +1100 Subject: [PATCH] [REF] Remove xssString as it is providing a false sense of security --- CRM/Core/Form.php | 1 - CRM/Utils/Rule.php | 24 +----------------------- api/v3/Generic/Setvalue.php | 3 --- api/v3/utils.php | 10 ---------- 4 files changed, 1 insertion(+), 37 deletions(-) diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index 28417580e350..8c49fee8a533 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -330,7 +330,6 @@ public function registerRules() { 'postalCode', 'money', 'positiveInteger', - 'xssString', 'fileExists', 'settingPath', 'autocomplete', diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php index 46402eebdca7..9341e173f922 100644 --- a/CRM/Utils/Rule.php +++ b/CRM/Utils/Rule.php @@ -801,22 +801,6 @@ public static function currencyCode($value) { return FALSE; } - /** - * @param $value - * - * @return bool - */ - public static function xssString($value) { - if (is_string($value)) { - return preg_match('!<(vb)?script[^>]*>.*!ims', - $value - ) ? FALSE : TRUE; - } - else { - return TRUE; - } - } - /** * Validate json string for xss * @@ -826,9 +810,6 @@ public static function xssString($value) { * False if invalid, true if valid / safe. */ public static function json($value) { - if (!self::xssString($value)) { - return FALSE; - } $array = json_decode($value, TRUE); if (!$array || !is_array($array)) { return FALSE; @@ -974,13 +955,10 @@ public static function checkExtensionKeyIsValid($key = NULL) { protected static function arrayValue($array) { foreach ($array as $key => $item) { if (is_array($item)) { - if (!self::xssString($key) || !self::arrayValue($item)) { + if (!self::arrayValue($item)) { return FALSE; } } - if (!self::xssString($key) || !self::xssString($item)) { - return FALSE; - } } return TRUE; } diff --git a/api/v3/Generic/Setvalue.php b/api/v3/Generic/Setvalue.php index b6ce00c9bc2b..8f6b10ed07eb 100644 --- a/api/v3/Generic/Setvalue.php +++ b/api/v3/Generic/Setvalue.php @@ -80,9 +80,6 @@ function civicrm_api3_generic_setValue($apiRequest) { case CRM_Utils_Type::T_STRING: case CRM_Utils_Type::T_TEXT: - if (!CRM_Utils_Rule::xssString($value)) { - return civicrm_api3_create_error(ts('Illegal characters in input (potential scripting attack)'), ['error_code' => 'XSS']); - } if (array_key_exists('maxlength', $def)) { $value = substr($value, 0, $def['maxlength']); } diff --git a/api/v3/utils.php b/api/v3/utils.php index 799dc7803197..86b374b4e6c0 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -2186,11 +2186,6 @@ function _civicrm_api3_validate_html(&$params, &$fieldName, $fieldInfo) { if (strpos($op, 'NULL') || strpos($op, 'EMPTY')) { return; } - if ($fieldValue) { - if (!CRM_Utils_Rule::xssString($fieldValue)) { - throw new API_Exception('Input contains illegal SCRIPT tag.', ["field" => $fieldName, "error_code" => "xss"]); - } - } } /** @@ -2219,11 +2214,6 @@ function _civicrm_api3_validate_string(&$params, &$fieldName, &$fieldInfo, $enti if ($fieldValue) { foreach ((array) $fieldValue as $key => $value) { - foreach ([$fieldValue, $key, $value] as $input) { - if (!CRM_Utils_Rule::xssString($input)) { - throw new Exception('Input contains illegal SCRIPT tag.'); - } - } if ($fieldName == 'currency') { //When using IN operator $fieldValue is a array of currency codes if (!CRM_Utils_Rule::currencyCode($value)) {