CVE-2020-15128: Use of a Broken or Risky Cryptographic Algorithm (CWE-327)
value: In OctoberCMS before version 1.0.468, encrypted cookie values were not tied to the name of the cookie the value belonged to. This meant that certain classes of attacks that took advantage of other theoretical vulnerabilities in user facing code (nothing exploitable in the core project itself) had a higher chance of succeeding. Specifically, if your usage exposed a way for users to provide unfiltered user input and have it returned to them as an encrypted cookie (ex. storing a user provided search query in a cookie) they could then use the generated cookie in place of other more tightly controlled cookies; or if your usage exposed the plaintext version of an encrypted cookie at any point to the user they could theoretically provide encrypted content from your application back to it as an encrypted cookie and force the framework to decrypt it for them. Issue has been fixed in build 468 (v1.0.468).
CWE description: The product uses a broken or risky cryptographic algorithm or protocol.
CVSS v3 base score: 6.3
Published: 2020-07-31T18:15Z
Repository: https://github.com/octobercms/library
Language: PHP
Files changed: src/Cookie/CookieValuePrefix.php, src/Cookie/Middleware/EncryptCookies.php
Commit date: 2020-07-28 21:05:38 -0600
Commit message: Improvements to cookie value validation (#508)


Diff: diff --git a/src/Cookie/CookieValuePrefix.php b/src/Cookie/CookieValuePrefix.php
new file mode 100644
index 00000000..e8076964
--- /dev/null
+++ b/src/Cookie/CookieValuePrefix.php
@@ -0,0 +1,47 @@
+<?php namespace October\Rain\Cookie;
+
+/**
+ * Helper class to prefix, unprefix, and verify cookie values
+ */
+class CookieValuePrefix
+{
+    /**
+     * Create a new cookie value prefix for the given cookie name.
+     *
+     * @param  string $name The name of the cookie
+     * @param  string $key The encryption key
+     * @return string
+     */
+    public static function create($name, $key)
+    {
+        return hash_hmac('sha1', $name . 'v2', $key) . '|';
+    }
+
+    /**
+     * Remove the cookie value prefix.
+     *
+     * @param  string  $cookieValue
+     * @return string
+     */
+    public static function remove($cookieValue)
+    {
+        return substr($cookieValue, 41);
+    }
+
+    /**
+     * Verify the provided cookie's value
+     *
+     * @param string $name The name of the cookie
+     * @param string $value The decrypted value of the cookie to be verified
+     * @param string $key The encryption key used to encrypt the cookie originally
+     * @return string|null $verifiedValue The unprefixed value if it passed verification, otherwise null
+     */
+    public static function getVerifiedValue($name, $value, $key)
+    {
+        $verifiedValue = null;
+        if (starts_with($value, static::create($name, $key))) {
+            $verifiedValue = static::remove($value);
+        }
+        return $verifiedValue;
+    }
+}
diff --git a/src/Cookie/Middleware/EncryptCookies.php b/src/Cookie/Middleware/EncryptCookies.php
index e64e2fd3..124d5783 100644
--- a/src/Cookie/Middleware/EncryptCookies.php
+++ b/src/Cookie/Middleware/EncryptCookies.php
@@ -1,8 +1,13 @@
 <?php namespace October\Rain\Cookie\Middleware;
 
 use Config;
+use Session;
+use October\Rain\Cookie\CookieValuePrefix;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Illuminate\Contracts\Encryption\DecryptException;
 use Illuminate\Contracts\Encryption\Encrypter as EncrypterContract;
 use Illuminate\Cookie\Middleware\EncryptCookies as EncryptCookiesBase;
 
 class EncryptCookies extends EncryptCookiesBase
 {
@@ -55,6 +60,74 @@ class EncryptCookies extends EncryptCookiesBase
             }
         }
 
         return $decrypted;
     }
+
+    /**
+     * Decrypt the cookies on the request.
+     *
+     * @param  \Symfony\Component\HttpFoundation\Request  $request
+     * @return \Symfony\Component\HttpFoundation\Request
+     */
+    protected function decrypt(Request $request)
+    {
+        foreach ($request->cookies as $key => $cookie) {
+            if ($this->isDisabled($key)) {
+                continue;
+            }
+
+            try {
+                // Decrypt the request-provided cookie
+                $decryptedValue = $this->decryptCookie($key, $cookie);
+
+                // Verify that the decrypted value belongs to this cookie key, use null if it fails
+                $value = CookieValuePrefix::getVerifiedValue($key, $decryptedValue, $this->encrypter->getKey());
+
+                /**
+                 * If the cookie is for the session and the value is a valid Session ID,
+                 * then allow it to pass through even if the validation failed (most likely
+                 * because the upgrade just occurred)
+                 *
+                 * The cookie will be adjusted on the next request
+                 * @todo Remove if year >= 2021 or build >= 475
+                 */
+                if (empty($value) && $key === Config::get('session.cookie') && Session::isValidId($decryptedValue)) {
+                    $value = $decryptedValue;
+                }
+
+                // Set the verified cookie value on the request
+                $request->cookies->set($key, $value);
+            } catch (DecryptException $e) {
+                $request->cookies->set($key, null);
+            }
+        }
+
+        return $request;
+    }
+
+    /**
+     * Encrypt the cookies on an outgoing response.
+     *
+     * @param  \Symfony\Component\HttpFoundation\Response  $response
+     * @return \Symfony\Component\HttpFoundation\Response
+     */
+    protected function encrypt(Response $response)
+    {
+        foreach ($response->headers->getCookies() as $cookie) {
+            if ($this->isDisabled($cookie->getName())) {
+                continue;
+            }
+
+            $response->headers->setCookie($this->duplicate(
+                $cookie,
+                $this->encrypter->encrypt(
+                    // Prefix the cookie value to verify that it belongs to the current cookie
+                    CookieValuePrefix::create($cookie->getName(), $this->encrypter->getKey()) . $cookie->getValue(),
+                    static::serialized($cookie->getName())
+                )
+            ));
+        }
+
+        return $response;
+    }
 }

Fixed code: <?php namespace October\Rain\Cookie;

/**
 * Helper class to prefix, unprefix, and verify cookie values
 */
class CookieValuePrefix
{
    /**
     * Create a new cookie value prefix for the given cookie name.
     *
     * @param  string $name The name of the cookie
     * @param  string $key The encryption key
     * @return string
     */
    public static function create($name, $key)
    {
        return hash_hmac('sha1', $name . 'v2', $key) . '|';
    }

    /**
     * Remove the cookie value prefix.
     *
     * @param  string  $cookieValue
     * @return string
     */
    public static function remove($cookieValue)
    {
        return substr($cookieValue, 41);
    }

    /**
     * Verify the provided cookie's value
     *
     * @param string $name The name of the cookie
     * @param string $value The decrypted value of the cookie to be verified
     * @param string $key The encryption key used to encrypt the cookie originally
     * @return string|null $verifiedValue The unprefixed value if it passed verification, otherwise null
     */
    public static function getVerifiedValue($name, $value, $key)
    {
        $verifiedValue = null;
        if (starts_with($value, static::create($name, $key))) {
            $verifiedValue = static::remove($value);
        }
        return $verifiedValue;
    }
}
use Session;
use October\Rain\Cookie\CookieValuePrefix;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Illuminate\Contracts\Encryption\DecryptException;

    /**
     * Decrypt the cookies on the request.
     *
     * @param  \Symfony\Component\HttpFoundation\Request  $request
     * @return \Symfony\Component\HttpFoundation\Request
     */
    protected function decrypt(Request $request)
    {
        foreach ($request->cookies as $key => $cookie) {
            if ($this->isDisabled($key)) {
                continue;
            }

            try {
                // Decrypt the request-provided cookie
                $decryptedValue = $this->decryptCookie($key, $cookie);

                // Verify that the decrypted value belongs to this cookie key, use null if it fails
                $value = CookieValuePrefix::getVerifiedValue($key, $decryptedValue, $this->encrypter->getKey());

                /**
                 * If the cookie is for the session and the value is a valid Session ID,
                 * then allow it to pass through even if the validation failed (most likely
                 * because the upgrade just occurred)
                 *
                 * The cookie will be adjusted on the next request
                 * @todo Remove if year >= 2021 or build >= 475
                 */
                if (empty($value) && $key === Config::get('session.cookie') && Session::isValidId($decryptedValue)) {
                    $value = $decryptedValue;
                }

                // Set the verified cookie value on the request
                $request->cookies->set($key, $value);
            } catch (DecryptException $e) {
                $request->cookies->set($key, null);
            }
        }

        return $request;
    }

    /**
     * Encrypt the cookies on an outgoing response.
     *
     * @param  \Symfony\Component\HttpFoundation\Response  $response
     * @return \Symfony\Component\HttpFoundation\Response
     */
    protected function encrypt(Response $response)
    {
        foreach ($response->headers->getCookies() as $cookie) {
            if ($this->isDisabled($cookie->getName())) {
                continue;
            }

            $response->headers->setCookie($this->duplicate(
                $cookie,
                $this->encrypter->encrypt(
                    // Prefix the cookie value to verify that it belongs to the current cookie
                    CookieValuePrefix::create($cookie->getName(), $this->encrypter->getKey()) . $cookie->getValue(),
                    static::serialized($cookie->getName())
                )
            ));
        }

        return $response;
    }