CVE-2023-2848: Origin Validation Error (CWE-346)
value: Movim prior to version 0.22 is affected by a Cross-Site WebSocket Hijacking vulnerability. This was the result of a missing header validation.
CWE description: The product does not properly verify that the source of data or communication is valid.
CVSS v3 base score: 8.8
Published: 2023-09-14T12:15Z
Repository: https://github.com/movim/movim
Language: PHP
Files changed: CHANGELOG.md, src/Movim/Daemon/Core.php
Commit date: 2023-05-07 14:24:28 +0200
Commit message: Add a same-origin check for non-internal connected Websockets

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8551e6da8..bb8b69ec1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,10 +5,11 @@ v0.22 (trunk)
 ---------------------------
  * Enable blog privacy feature, can be switched between private (presence only) and open
  * Fix vcard-temp refresh
  * Fix #1177 Refresh Ad-Hoc commande list when command finished
  * Fix #1163 Ad-hoc command returns only one item (Prosŏdy)
+ * Add a same-origin check for non-internal connected Websockets
 
 v0.21.1
 ---------------------------
  * Fix #1183 compileOpcache fails with php8.1
  * Only load required database so when launching sessions
diff --git a/src/Movim/Daemon/Core.php b/src/Movim/Daemon/Core.php
index e5ba66c57..0869f00a6 100644
--- a/src/Movim/Daemon/Core.php
+++ b/src/Movim/Daemon/Core.php
@@ -52,30 +52,30 @@ class Core implements MessageComponentInterface
 
         $this->registerCleaner();
 
         // Generate Push Notification
         if (!file_exists(CACHE_PATH . 'vapid_keys.json')) {
-            echo colorize("Generate and store the Push Notification VAPID keys", 'green')."\n";
+            echo colorize("Generate and store the Push Notification VAPID keys", 'green') . "\n";
             $keyset = VAPID::createVapidKeys();
             file_put_contents(CACHE_PATH . 'vapid_keys.json', json_encode($keyset));
         }
     }
 
     public function setWebsocket($port)
     {
         echo
-            "\n".
-            "--- ".colorize("Server Configuration - Apache", 'purple')." ---".
+        "\n" .
+            "--- " . colorize("Server Configuration - Apache", 'purple') . " ---" .
             "\n";
-        echo colorize("Enable the Secure WebSocket to WebSocket tunneling", 'yellow')."\n# a2enmod proxy_wstunnel \n";
-        echo colorize("Add this in your configuration file (default-ssl.conf)", 'yellow')."\nProxyPass /ws/ ws://127.0.0.1:{$port}/\n";
+        echo colorize("Enable the Secure WebSocket to WebSocket tunneling", 'yellow') . "\n# a2enmod proxy_wstunnel \n";
+        echo colorize("Add this in your configuration file (default-ssl.conf)", 'yellow') . "\nProxyPass /ws/ ws://127.0.0.1:{$port}/\n";
 
         echo
-            "\n".
-            "--- ".colorize("Server Configuration - nginx", 'purple')." ---".
+        "\n" .
+            "--- " . colorize("Server Configuration - nginx", 'purple') . " ---" .
             "\n";
-        echo colorize("Add this in your configuration file", 'yellow')."\n";
+        echo colorize("Add this in your configuration file", 'yellow') . "\n";
         echo "location /ws/ {
     proxy_pass http://127.0.0.1:{$port}/;
     proxy_http_version 1.1;
     proxy_set_header Upgrade \$http_upgrade;
     proxy_set_header Connection \"Upgrade\";
@@ -86,30 +86,35 @@ class Core implements MessageComponentInterface
     proxy_redirect off;
 }
 ";
 
         echo
-            "\n".
-            "--- ".colorize("Server Configuration - Caddy", 'purple')." ---".
+        "\n" .
+            "--- " . colorize("Server Configuration - Caddy", 'purple') . " ---" .
             "\n";
-            echo colorize("Add this in your configuration file", 'yellow')."\nhandle /ws/* {
+        echo colorize("Add this in your configuration file", 'yellow') . "\nhandle /ws/* {
     reverse_proxy localhost:8080
 }
 
 ";
     }
 
     public function onOpen(ConnectionInterface $conn)
     {
+        if (!$this->isTrustedConnection($conn)) $conn->close();
+
         // WebSockets from the Browser
         $sid = $this->getSid($conn);
+
         if ($sid != null) {
             $path = $this->getPath($conn);
 
             if (in_array($path, $this->single)) {
-                if (array_key_exists($sid, $this->singlelocks)
-                && array_key_exists($path, $this->singlelocks[$sid])) {
+                if (
+                    array_key_exists($sid, $this->singlelocks)
+                    && array_key_exists($path, $this->singlelocks[$sid])
+                ) {
                     $this->singlelocks[$sid][$path]++;
                     $conn->close(1008);
                 } else {
                     $this->singlelocks[$sid][$path] = 1;
                 }
@@ -165,12 +170,14 @@ class Core implements MessageComponentInterface
 
         if ($sid != null && isset($this->sessions[$sid])) {
             $path = $this->getPath($conn);
 
             if (in_array($path, $this->single)) {
-                if (array_key_exists($sid, $this->singlelocks)
-                && array_key_exists($path, $this->singlelocks[$sid])) {
+                if (
+                    array_key_exists($sid, $this->singlelocks)
+                    && array_key_exists($path, $this->singlelocks[$sid])
+                ) {
                     $this->singlelocks[$sid][$path]--;
                     if ($this->singlelocks[$sid][$path] == 0) {
                         unset($this->singlelocks[$sid][$path]);
                     }
                 }
@@ -195,12 +202,14 @@ class Core implements MessageComponentInterface
 
     private function registerCleaner()
     {
         $this->loop->addPeriodicTimer(5, function () {
             foreach ($this->sessions as $sid => $session) {
-                if ($session->countClients() == 0
-                && $session->registered == null) {
+                if (
+                    $session->countClients() == 0
+                    && $session->registered == null
+                ) {
                     $session->killLinker();
                 }
 
                 if ($session->process == null) {
                     unset($this->sessions[$sid]);
@@ -214,29 +223,29 @@ class Core implements MessageComponentInterface
     }
 
     private function cleanupDBSessions()
     {
         DBSession::where('active', false)
-            ->where('created_at', '<', date(MOVIM_SQL_DATE, time()-60))
+            ->where('created_at', '<', date(MOVIM_SQL_DATE, time() - 60))
             ->delete();
     }
 
     /**
      * @desc Delete push subscriptions without activity after a month
      */
     private function cleanupPushSubscriptions()
     {
-        PushSubscription::where('activity_at', '<', date(MOVIM_SQL_DATE, time()-(60*60*24*30)))
+        PushSubscription::where('activity_at', '<', date(MOVIM_SQL_DATE, time() - (60 * 60 * 24 * 30)))
             ->delete();
     }
 
     /**
      * @desc Delete encrypted passwords after 7 days without update
      */
     private function cleanupEncryptedPasswords()
     {
-        EncryptedPassword::where('updated_at', '<', date(MOVIM_SQL_DATE, time()-(60*60*24*7)))
+        EncryptedPassword::where('updated_at', '<', date(MOVIM_SQL_DATE, time() - (60 * 60 * 24 * 7)))
             ->delete();
     }
 
     public function onError(ConnectionInterface $conn, \Exception $e)
     {
@@ -265,11 +274,11 @@ class Core implements MessageComponentInterface
     }
 
     private function getOffset(ConnectionInterface $conn)
     {
         parse_str($conn->httpRequest->getUri()->getQuery(), $arr);
-        return (isset($arr['offset'])) ? invertSign(((int)$arr['offset'])*60) : 0;
+        return (isset($arr['offset'])) ? invertSign(((int)$arr['offset']) * 60) : 0;
     }
 
     private function getPath(ConnectionInterface $conn)
     {
         parse_str($conn->httpRequest->getUri()->getQuery(), $arr);
@@ -285,11 +294,19 @@ class Core implements MessageComponentInterface
             : null;
     }
 
     private function getHeaderSid(ConnectionInterface $conn)
     {
-        return ($conn->httpRequest->hasHeader('MOVIM_SESSION_ID')
-            && $conn->httpRequest->getHeader('MOVIM_DAEMON_KEY')[0] === $this->key)
+        return ($conn->httpRequest->hasHeader('MOVIM_SESSION_ID'))
             ? $conn->httpRequest->getHeader('MOVIM_SESSION_ID')[0]
             : null;
     }
+
+    private function isTrustedConnection(ConnectionInterface $conn): bool
+    {
+        $daemonKeyHeader = $conn->httpRequest->getHeader('MOVIM_DAEMON_KEY');
+        $secFetchSiteHeader = $conn->httpRequest->getHeader('Sec-Fetch-Site');
+
+        return (is_array($daemonKeyHeader) && !empty($daemonKeyHeader) && $daemonKeyHeader[0] === $this->key)
+            || (is_array($secFetchSiteHeader) && !empty($secFetchSiteHeader) && $secFetchSiteHeader[0] == 'same-origin');
+    }
 }

Vulnerable code:             echo colorize("Generate and store the Push Notification VAPID keys", 'green')."\n";
            "\n".
            "--- ".colorize("Server Configuration - Apache", 'purple')." ---".
        echo colorize("Enable the Secure WebSocket to WebSocket tunneling", 'yellow')."\n# a2enmod proxy_wstunnel \n";
        echo colorize("Add this in your configuration file (default-ssl.conf)", 'yellow')."\nProxyPass /ws/ ws://127.0.0.1:{$port}/\n";
            "\n".
            "--- ".colorize("Server Configuration - nginx", 'purple')." ---".
        echo colorize("Add this in your configuration file", 'yellow')."\n";
            "\n".
            "--- ".colorize("Server Configuration - Caddy", 'purple')." ---".
            echo colorize("Add this in your configuration file", 'yellow')."\nhandle /ws/* {
                if (array_key_exists($sid, $this->singlelocks)
                && array_key_exists($path, $this->singlelocks[$sid])) {
                if (array_key_exists($sid, $this->singlelocks)
                && array_key_exists($path, $this->singlelocks[$sid])) {
                if ($session->countClients() == 0
                && $session->registered == null) {
            ->where('created_at', '<', date(MOVIM_SQL_DATE, time()-60))
        PushSubscription::where('activity_at', '<', date(MOVIM_SQL_DATE, time()-(60*60*24*30)))
        EncryptedPassword::where('updated_at', '<', date(MOVIM_SQL_DATE, time()-(60*60*24*7)))
        return (isset($arr['offset'])) ? invertSign(((int)$arr['offset'])*60) : 0;
        return ($conn->httpRequest->hasHeader('MOVIM_SESSION_ID')
            && $conn->httpRequest->getHeader('MOVIM_DAEMON_KEY')[0] === $this->key)
Fixed code:  * Add a same-origin check for non-internal connected Websockets
            echo colorize("Generate and store the Push Notification VAPID keys", 'green') . "\n";
        "\n" .
            "--- " . colorize("Server Configuration - Apache", 'purple') . " ---" .
        echo colorize("Enable the Secure WebSocket to WebSocket tunneling", 'yellow') . "\n# a2enmod proxy_wstunnel \n";
        echo colorize("Add this in your configuration file (default-ssl.conf)", 'yellow') . "\nProxyPass /ws/ ws://127.0.0.1:{$port}/\n";
        "\n" .
            "--- " . colorize("Server Configuration - nginx", 'purple') . " ---" .
        echo colorize("Add this in your configuration file", 'yellow') . "\n";
        "\n" .
            "--- " . colorize("Server Configuration - Caddy", 'purple') . " ---" .
        echo colorize("Add this in your configuration file", 'yellow') . "\nhandle /ws/* {
        if (!$this->isTrustedConnection($conn)) $conn->close();


                if (
                    array_key_exists($sid, $this->singlelocks)
                    && array_key_exists($path, $this->singlelocks[$sid])
                ) {
                if (
                    array_key_exists($sid, $this->singlelocks)
                    && array_key_exists($path, $this->singlelocks[$sid])
                ) {
                if (
                    $session->countClients() == 0
                    && $session->registered == null
                ) {
            ->where('created_at', '<', date(MOVIM_SQL_DATE, time() - 60))
        PushSubscription::where('activity_at', '<', date(MOVIM_SQL_DATE, time() - (60 * 60 * 24 * 30)))
        EncryptedPassword::where('updated_at', '<', date(MOVIM_SQL_DATE, time() - (60 * 60 * 24 * 7)))
        return (isset($arr['offset'])) ? invertSign(((int)$arr['offset']) * 60) : 0;
        return ($conn->httpRequest->hasHeader('MOVIM_SESSION_ID'))

    private function isTrustedConnection(ConnectionInterface $conn): bool
    {
        $daemonKeyHeader = $conn->httpRequest->getHeader('MOVIM_DAEMON_KEY');
        $secFetchSiteHeader = $conn->httpRequest->getHeader('Sec-Fetch-Site');

        return (is_array($daemonKeyHeader) && !empty($daemonKeyHeader) && $daemonKeyHeader[0] === $this->key)
            || (is_array($secFetchSiteHeader) && !empty($secFetchSiteHeader) && $secFetchSiteHeader[0] == 'same-origin');
    }