From 3b93636aa7aea07d1f7dfa36b63b773ac16d7cde Mon Sep 17 00:00:00 2001
From: Dan Garner <dan@xibosignage.com>
Date: Mon, 18 Mar 2024 12:43:29 +0000
Subject: [PATCH] Improvements to filtering IP and sessions (#2434)

* Sessions: Add sanitization in getIp() and disable output of session ID in session grid. xibosignage/xibo#3375
* Bump to 3.3.10
---
 lib/Controller/Sessions.php             | 28 +++++++++--------------
 lib/Factory/SessionFactory.php          | 30 +++++++++++++------------
 lib/Helper/Environment.php              |  2 +-
 lib/Helper/Session.php                  | 13 ++++++-----
 lib/Xmds/Soap.php                       |  6 ++---
 views/sessions-form-confirm-logout.twig |  2 +-
 6 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/lib/Controller/Sessions.php b/lib/Controller/Sessions.php
index b4c872d8e2..09ee537ad5 100644
--- a/lib/Controller/Sessions.php
+++ b/lib/Controller/Sessions.php
@@ -1,8 +1,8 @@
 <?php
-/**
- * Copyright (C) 2021 Xibo Signage Ltd
+/*
+ * Copyright (C) 2024 Xibo Signage Ltd
  *
- * Xibo - Digital Signage - http://www.xibo.org.uk
+ * Xibo - Digital Signage - https://xibosignage.com
  *
  * This file is part of Xibo.
  *
@@ -22,7 +22,6 @@
 namespace Xibo\Controller;
 
 use Carbon\Carbon;
-
 use Slim\Http\Response as Response;
 use Slim\Http\ServerRequest as Request;
 use Xibo\Factory\SessionFactory;
@@ -94,13 +93,16 @@ function grid(Request $request, Response $response)
             $row->lastAccessed = Carbon::createFromTimeString($row->lastAccessed)->format(DateFormatHelper::getSystemFormat());
 
             if (!$this->isApi($request) && $this->getUser()->isSuperAdmin()) {
-
                 $row->includeProperty('buttons');
 
+                if ($row->isExpired == 1) {
+                    continue;
+                }
+
                 // Edit
                 $row->buttons[] = array(
                     'id' => 'sessions_button_logout',
-                    'url' => $this->urlFor($request,'sessions.confirm.logout.form', ['id' => $row->sessionId]),
+                    'url' => $this->urlFor($request,'sessions.confirm.logout.form', ['id' => $row->userId]),
                     'text' => __('Logout')
                 );
             }
@@ -131,7 +133,7 @@ function confirmLogoutForm(Request $request, Response $response, $id)
 
         $this->getState()->template = 'sessions-form-confirm-logout';
         $this->getState()->setData([
-            'sessionId' => $id,
+            'userId' => $id,
             'help' => $this->getHelp()->link('Sessions', 'Logout')
         ]);
 
@@ -149,21 +151,13 @@ function confirmLogoutForm(Request $request, Response $response, $id)
      * @throws \Xibo\Support\Exception\GeneralException
      * @throws \Xibo\Support\Exception\NotFoundException
      */
-    function logout(Request $request, Response $response, $id)
+    public function logout(Request $request, Response $response, $id)
     {
         if ($this->getUser()->userTypeId != 1) {
             throw new AccessDeniedException();
         }
 
-        $session = $this->sessionFactory->getById($id);
-
-        if ($session->userId != 0) {
-            $this->store->update('UPDATE `session` SET IsExpired = 1 WHERE userID = :userId ',
-                ['userId' => $session->userId]);
-        } else {
-            $this->store->update('UPDATE `session` SET IsExpired = 1 WHERE session_id = :session_id ',
-                ['session_id' => $id]);
-        }
+        $this->sessionFactory->expireByUserId($id);
 
         // Return
         $this->getState()->hydrate([
diff --git a/lib/Factory/SessionFactory.php b/lib/Factory/SessionFactory.php
index 5626d82178..62774ce85c 100644
--- a/lib/Factory/SessionFactory.php
+++ b/lib/Factory/SessionFactory.php
@@ -1,8 +1,8 @@
 <?php
 /*
- * Copyright (c) 2022 Xibo Signage Ltd
+ * Copyright (C) 2024 Xibo Signage Ltd
  *
- * Xibo - Digital Signage - http://www.xibo.org.uk
+ * Xibo - Digital Signage - https://xibosignage.com
  *
  * This file is part of Xibo.
  *
@@ -26,7 +26,6 @@
 
 use Xibo\Entity\Session;
 use Xibo\Helper\DateFormatHelper;
-use Xibo\Support\Exception\NotFoundException;
 
 /**
  * Class SessionFactory
@@ -43,18 +42,14 @@ public function createEmpty()
     }
 
     /**
-     * @param $sessionId
-     * @return Session
-     * @throws NotFoundException
+     * @param int $userId
      */
-    public function getById($sessionId)
+    public function expireByUserId($userId): void
     {
-        $session = $this->query(null, ['sessionId' => $sessionId]);
-
-        if (count($session) <= 0)
-            throw new NotFoundException();
-
-        return $session[0];
+        $this->getStore()->update(
+            'UPDATE `session` SET IsExpired = 1 WHERE userID = :userId ',
+            ['userId' => $userId]
+        );
     }
 
     /**
@@ -143,7 +138,14 @@ public function query($sortOrder = null, $filterBy = [])
 
 
         foreach ($this->getStore()->select($sql, $params) as $row) {
-            $entries[] = $this->createEmpty()->hydrate($row, ['stringProperties' => ['sessionId']]);
+            $session = $this->createEmpty()->hydrate($row, [
+                'stringProperties' => ['sessionId'],
+                'intProperties' => ['isExpired']
+            ]);
+            $session->userAgent = htmlspecialchars($session->userAgent);
+            $session->remoteAddress = filter_var($session->remoteAddress, FILTER_VALIDATE_IP);
+            $session->excludeProperty('sessionId');
+            $entries[] = $session;
         }
 
         // Paging
diff --git a/lib/Helper/Environment.php b/lib/Helper/Environment.php
index 64d160644e..d91790aa5c 100644
--- a/lib/Helper/Environment.php
+++ b/lib/Helper/Environment.php
@@ -30,7 +30,7 @@
  */
 class Environment
 {
-    public static $WEBSITE_VERSION_NAME = '3.3.9';
+    public static $WEBSITE_VERSION_NAME = '3.3.10';
     public static $XMDS_VERSION = '6';
     public static $XLF_VERSION = 3;
     public static $VERSION_REQUIRED = '7.2.9';
diff --git a/lib/Helper/Session.php b/lib/Helper/Session.php
index 44b6b1ff13..4f96433a14 100644
--- a/lib/Helper/Session.php
+++ b/lib/Helper/Session.php
@@ -1,14 +1,15 @@
 <?php
 /*
- * Xibo - Digital Signage - http://www.xibo.org.uk
- * Copyright (C) 2006-2018 Spring Signage Ltd
+ * Copyright (C) 2024 Xibo Signage Ltd
+ *
+ * Xibo - Digital Signage - https://xibosignage.com
  *
  * This file is part of Xibo.
  *
  * Xibo is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Affero General Public License as published by
  * the Free Software Foundation, either version 3 of the License, or
- * any later version. 
+ * any later version.
  *
  * Xibo is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -171,7 +172,7 @@ function read($key)
         $data = '';
         $this->key = $key;
 
-        $userAgent = substr($_SERVER['HTTP_USER_AGENT'], 0, 253);
+        $userAgent = substr(htmlspecialchars($_SERVER['HTTP_USER_AGENT']), 0, 253);
 
         try {
             $dbh = $this->getDb();
@@ -421,7 +422,7 @@ private function insertSession($key, $data, $lastAccessed, $expiry)
             'lastAccessed' => Carbon::createFromTimestamp($lastAccessed)->format(DateFormatHelper::getSystemFormat()),
             'userId' => $this->userId,
             'expired' => ($this->expired) ? 1 : 0,
-            'useragent' => substr($_SERVER['HTTP_USER_AGENT'], 0, 253),
+            'useragent' => substr(htmlspecialchars($_SERVER['HTTP_USER_AGENT']), 0, 253),
             'remoteaddr' => $this->getIp()
         ];
 
@@ -470,7 +471,7 @@ private function getIp()
         $clientIp = '';
         $keys = array('X_FORWARDED_FOR', 'HTTP_X_FORWARDED_FOR', 'CLIENT_IP', 'REMOTE_ADDR');
         foreach ($keys as $key) {
-            if (isset($_SERVER[$key])) {
+            if (isset($_SERVER[$key]) && filter_var($_SERVER[$key], FILTER_VALIDATE_IP) !== false) {
                 $clientIp = $_SERVER[$key];
                 break;
             }
diff --git a/lib/Xmds/Soap.php b/lib/Xmds/Soap.php
index 145be59871..60a5394dd1 100644
--- a/lib/Xmds/Soap.php
+++ b/lib/Xmds/Soap.php
@@ -1,8 +1,8 @@
 <?php
 /*
- * Copyright (C) 2023 Xibo Signage Ltd
+ * Copyright (C) 2024 Xibo Signage Ltd
  *
- * Xibo - Digital Signage - http://www.xibo.org.uk
+ * Xibo - Digital Signage - https://xibosignage.com
  *
  * This file is part of Xibo.
  *
@@ -2230,7 +2230,7 @@ protected function getIp()
 
         $keys = array('X_FORWARDED_FOR', 'HTTP_X_FORWARDED_FOR', 'CLIENT_IP', 'REMOTE_ADDR');
         foreach ($keys as $key) {
-            if (isset($_SERVER[$key])) {
+            if (isset($_SERVER[$key]) && filter_var($_SERVER[$key], FILTER_VALIDATE_IP) !== false) {
                 $clientIp = $_SERVER[$key];
                 break;
             }
diff --git a/views/sessions-form-confirm-logout.twig b/views/sessions-form-confirm-logout.twig
index dd7dacb59f..57e51c7e00 100644
--- a/views/sessions-form-confirm-logout.twig
+++ b/views/sessions-form-confirm-logout.twig
@@ -37,7 +37,7 @@
 {% block formHtml %}
     <div class="row">
         <div class="col-md-12">
-            <form id="sessionsConfirmLogoutForm" class="XiboForm form-horizontal" method="delete" action="{{ url_for("sessions.confirm.logout", {id: sessionId}) }}">
+            <form id="sessionsConfirmLogoutForm" class="XiboForm form-horizontal" method="delete" action="{{ url_for("sessions.confirm.logout", {id: userId}) }}">
                 {% set message %}{% trans "Are you sure you want to logout this user?" %}{% endset %}
                 {{ forms.message(message) }}
             </form>