Merge pull request #18007 from demeritcowboy/strpos-revisited

dev/core#1918 - Remove dubious qfkey checking code that never runs

CRM/Core/Key.php

@@ -110,30 +110,19 @@ public static function validate($key, $name, $addSequence = FALSE) {
   }
 
   /**
-   * @param $key
+   * The original version of this function, added circa 2010 and untouched
+   * since then, seemed intended to check for a 32-digit hex string followed
+   * optionally by an underscore and 4-digit number. But it had a bug where
+   * the optional part was never checked ever. So have decided to remove that
+   * second check to keep it simple since it seems like pseudo-security.
+   *
+   * @param string $key
    *
    * @return bool
    */
   public static function valid($key) {
-    // a valid key is a 32 digit hex number
-    // followed by an optional _ and a number between 1 and 10000
-    if (strpos('_', $key) !== FALSE) {
-      list($hash, $seq) = explode('_', $key);
-
-      // ensure seq is between 1 and 10000
-      if (!is_numeric($seq) ||
-        $seq < 1 ||
-        $seq > 10000
-      ) {
-        return FALSE;
-      }
-    }
-    else {
-      $hash = $key;
-    }
-
-    // ensure that hash is a 32 digit hex number
-    return (bool) preg_match('#[0-9a-f]{32}#i', $hash);
+    // ensure that key contains a 32 digit hex string
+    return (bool) preg_match('#[0-9a-f]{32}#i', $key);
   }
 
 }