Skip to content

Commit

Permalink
Improvements to filtering IP and sessions (#2433)
Browse files Browse the repository at this point in the history
* Sessions: Add sanitization in getIp() and disable output of session ID in session grid. xibosignage/xibo#3375
* Bump to 4.0.9
  • Loading branch information
dasgarner committed Mar 18, 2024
1 parent fa1da3f commit 49f018f
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 39 deletions.
30 changes: 12 additions & 18 deletions lib/Controller/Sessions.php
Expand Up @@ -96,6 +96,11 @@ public function grid(Request $request, Response $response): Response|\Psr\Http\M
if (!$this->isApi($request) && $this->getUser()->isSuperAdmin()) {
$row->includeProperty('buttons');

// No buttons on expired sessions
if ($row->isExpired == 1) {
continue;
}

// logout, current user/session
if ($row->userId === $this->getUser()->userId && session_id() === $row->sessionId) {
$url = $this->urlFor($request, 'logout');
Expand All @@ -104,7 +109,7 @@ public function grid(Request $request, Response $response): Response|\Psr\Http\M
$url = $this->urlFor(
$request,
'sessions.confirm.logout.form',
['id' => $row->sessionId]
['id' => $row->userId]
);
}

Expand All @@ -127,21 +132,21 @@ public function grid(Request $request, Response $response): Response|\Psr\Http\M
* Confirm Logout Form
* @param Request $request
* @param Response $response
* @param $id
* @param int $id The UserID
* @return \Psr\Http\Message\ResponseInterface|Response
* @throws AccessDeniedException
* @throws \Xibo\Support\Exception\ControllerNotImplemented
* @throws \Xibo\Support\Exception\GeneralException
*/
function confirmLogoutForm(Request $request, Response $response, $id)
public function confirmLogoutForm(Request $request, Response $response, $id)
{
if ($this->getUser()->userTypeId != 1) {
throw new AccessDeniedException();
}

$this->getState()->template = 'sessions-form-confirm-logout';
$this->getState()->setData([
'sessionId' => $id,
'userId' => $id,
]);

return $this->render($request, $response);
Expand All @@ -158,25 +163,14 @@ 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]
);
}
// We log out all of this user's sessions.
$this->sessionFactory->expireByUserId($id);

// Return
$this->getState()->hydrate([
Expand Down
30 changes: 16 additions & 14 deletions 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.
*
Expand All @@ -26,7 +26,6 @@

use Xibo\Entity\Session;
use Xibo\Helper\DateFormatHelper;
use Xibo\Support\Exception\NotFoundException;

/**
* Class SessionFactory
Expand All @@ -43,18 +42,14 @@ public function createEmpty()
}

/**
* @param $sessionId
* @return Session
* @throws NotFoundException
* @param int $userId
*/
public function getById($sessionId)
public function expireByUserId(int $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]
);
}

/**
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Helper/Environment.php
Expand Up @@ -30,7 +30,7 @@
*/
class Environment
{
public static $WEBSITE_VERSION_NAME = '4.0.8';
public static $WEBSITE_VERSION_NAME = '4.0.9';
public static $XMDS_VERSION = '7';
public static $XLF_VERSION = 4;
public static $VERSION_REQUIRED = '8.1.0';
Expand Down
8 changes: 4 additions & 4 deletions lib/Helper/Session.php
@@ -1,6 +1,6 @@
<?php
/*
* Copyright (C) 2023 Xibo Signage Ltd
* Copyright (C) 2024 Xibo Signage Ltd
*
* Xibo - Digital Signage - https://xibosignage.com
*
Expand Down Expand Up @@ -172,7 +172,7 @@ public function read($key): false|string
$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();
Expand Down Expand Up @@ -422,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()
];

Expand Down Expand Up @@ -471,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;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Xmds/Soap.php
Expand Up @@ -2640,7 +2640,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;
}
Expand Down
2 changes: 1 addition & 1 deletion views/sessions-form-confirm-logout.twig
Expand Up @@ -36,7 +36,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>
Expand Down

0 comments on commit 49f018f

Please sign in to comment.