Merge pull request #19045 from seamuslee001/remove_xss_function

[REF] Remove xssString as it is providing a false sense of security
civicrm · Nov 26, 2020 · ab8c4a3 · ab8c4a3
2 parents 309de23 + d33b612
commit ab8c4a3
Show file tree

Hide file tree

Showing 4 changed files with 1 addition and 37 deletions.
diff --git 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
@@ -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[^>]*>.*</(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
@@ -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
@@ -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)) {