Merge pull request #19211 from eileenmcnaughton/form_move

[civicrm-core.git] / CRM / Core / Key.php

diff --git a/CRM/Core/Key.php b/CRM/Core/Key.php
index 688c0f8cffdc31b5fb71b4f2612d6659bfb39e5d..3e7826f2c7172ae2691330defc261b5606da096a 100644 (file)
--- a/CRM/Core/Key.php
+++ b/CRM/Core/Key.php
@@ -32,10 +32,10 @@ class CRM_Core_Key {
   const HASH_ALGO = 'sha256';
 
   /**
-   * The length of a generated signature/digest (expressed in hex digits).
+   * The minimum length of a generated signature/digest (expressed in base36 digits).
    * @var int
    */
-  const HASH_LENGTH = 64;
+  const HASH_LENGTH = 25;
 
   public static $_key = NULL;
 
@@ -89,7 +89,7 @@ class CRM_Core_Key {
     $key = self::sign($name);
 
     if ($addSequence) {
-      // now generate a random number between 1 and 100K and add it to the key
+      // now generate a random number between 1 and 10000 and add it to the key
       // so that we can have forms in mutiple tabs etc
       $key = $key . '_' . mt_rand(1, 10000);
     }
@@ -121,18 +121,16 @@ class CRM_Core_Key {
       $k = $key;
     }
 
-    if (!hash_equals($k, self::sign($name))) {
+    $expected = self::sign($name);
+    if (!hash_equals($k, $expected)) {
       return NULL;
     }
     return $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.
+   * Check that the key is well-formed. This does not check that the key is
+   * currently a key that is in use or belongs to a real form/session.
    *
    * @param string $key
    *
@@ -140,8 +138,9 @@ class CRM_Core_Key {
    *   TRUE if the signature ($key) is well-formed.
    */
   public static function valid($key) {
-    // ensure that hash is a hex number (of expected length)
-    return preg_match('#[0-9a-f]{' . self::HASH_LENGTH . '}#i', $key) ? TRUE : FALSE;
+    // ensure that key is an alphanumeric string of at least HASH_LENGTH with
+    // an optional underscore+digits at the end.
+    return preg_match('#^[0-9a-zA-Z]{' . self::HASH_LENGTH . ',}+(_\d+)?$#', $key) ? TRUE : FALSE;
   }
 
   /**
@@ -157,8 +156,10 @@ class CRM_Core_Key {
     if (strpos($sessionID, $delim) !== FALSE || strpos($name, $delim) !== FALSE) {
       throw new \RuntimeException("Failed to generate signature. Malformed session-id or form-name.");
     }
+    // The "prefix" gives some advisory details to help with debugging.
+    $prefix = preg_replace('/[^a-zA-Z0-9]/', '', $name);
     // Note: Unsure why $sessionID is included, but it's always been there, and it doesn't seem harmful.
-    return hash_hmac(self::HASH_ALGO, $sessionID . $delim . $name, $privateKey);
+    return $prefix . base_convert(hash_hmac(self::HASH_ALGO, $sessionID . $delim . $name, $privateKey), 16, 36);
 
   }