Merge pull request #6356 from JediKev/security/refresh-url

Reviewed-By: JediKev <kevin@enhancesoft.com>, protich <peter@osticket.com>
osTicket · Nov 8, 2022 · dc0507e · dc0507e
2 parents 75e111c + 3702a4f
commit dc0507e
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 48 deletions.
diff --git a/bootstrap.php b/bootstrap.php
@@ -375,7 +375,7 @@ static function croak($message) {
 Bootstrap::init();
 
 #CURRENT EXECUTING SCRIPT.
-define('THISPAGE', Misc::currentURL());
+define('THISPAGE', Http::url());
 
 define('DEFAULT_MAX_FILE_UPLOADS', ini_get('max_file_uploads') ?: 5);
 define('DEFAULT_PRIORITY_ID', 1);

diff --git a/file.php b/file.php
@@ -46,8 +46,7 @@
 
     // Try and determine if an agent is viewing the page / file
     if (strpos($_SERVER['HTTP_REFERRER'], ROOT_PATH .  'scp/') !== false) {
-        $_SESSION['_staff']['auth']['dest'] =
-            '/' . ltrim($_SERVER['REQUEST_URI'], '/');
+        $_SESSION['_staff']['auth']['dest'] = Http::refresh_url();
         Http::redirect(ROOT_PATH.'scp/login.php');
     } else {
         require 'secure.inc.php';

diff --git a/include/class.format.php b/include/class.format.php
@@ -407,6 +407,14 @@ static function htmldecode($var) {
         return htmlspecialchars_decode($var, $flags);
     }
 
+    static function http_query_string(string $query, array $filter = null) {
+        $args = [];
+        parse_str($query, $args);
+        if ($filter && is_array($filter))
+            $args = array_diff_key($args, array_flip($filter));
+        return http_build_query($args);
+    }
+
     static function input($var) {
         return Format::htmlchars($var);
     }

diff --git a/include/class.http.php b/include/class.http.php
@@ -151,5 +151,48 @@ static function build_query($vars, $encode=true, $separator='&amp;') {
 
         return http_build_query($vars, '', $separator);
     }
+
+    // Get current url
+    static function url() {
+        // Scheme + Host.
+        $https =  osTicket::is_https();
+        $url  = sprintf('http%s://%s',
+                $https ? 's' : '',
+                $_SERVER['HTTP_HOST']);
+        //  Add Port if not 80 and not https
+        $port = osTicket::get_client_port();
+        if ($port && $port != 80 && !$https && !str_contains($url, $port))
+            $url .= ":{$port}";
+        // Path + Query string
+        if (isset($_SERVER['REQUEST_URI']))
+            $url  .= $_SERVER['REQUEST_URI'];
+        else
+            $url  .= sprintf('%s?$s',
+                    $_SERVER['PHP_SELF'], $_SERVER['QUERY_STRING']);
+
+        return $url;
+    }
+
+    // Parse path from given url or the current url
+    static function url_path(string $url = null, bool $htmlentities = true) {
+        $path = parse_url($url ?: self::url(), PHP_URL_PATH);
+        return ($htmlentities && $path)
+            ? htmlentities($path) : $path;
+    }
+
+    // Parse query string from current url
+    static function query_string(string $url = null) {
+        return parse_url($url ?: self::url(), PHP_URL_QUERY);
+    }
+
+    // Refresh url
+    static function refresh_url(array $queryFilter = []) {
+        $url = self::url();
+        $refresh_url = self::url_path($url);
+        if (($qs=self::query_string($url)))
+            $refresh_url .= sprintf('?%s',
+                    Format::http_query_string($qs, $queryFilter));
+        return $refresh_url;
+    }
 }
 ?>
diff --git a/include/class.misc.php b/include/class.misc.php
@@ -226,29 +226,6 @@ static function date_range($period, $time=false, $tz=null) {
         return (object) array('start' => $start, 'end' => $end);
     }
 
-    //Current page
-    static function currentURL() {
-
-        $str = 'http';
-        if (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') {
-            $str .='s';
-        }
-        $str .= '://';
-        if (!isset($_SERVER['REQUEST_URI'])) { //IIS???
-            $_SERVER['REQUEST_URI'] = substr($_SERVER['PHP_SELF'],1 );
-            if (isset($_SERVER['QUERY_STRING'])) {
-                $_SERVER['REQUEST_URI'].='?'.$_SERVER['QUERY_STRING'];
-            }
-        }
-        if ($_SERVER['SERVER_PORT']!=80) {
-            $str .= $_SERVER['SERVER_NAME'].':'.$_SERVER['SERVER_PORT'].$_SERVER['REQUEST_URI'];
-        } else {
-            $str .= $_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'];
-        }
-
-        return $str;
-    }
-
     static function realpath($path) {
         $rp = realpath($path);
         return $rp ? $rp : $path;

diff --git a/include/class.osticket.php b/include/class.osticket.php
@@ -520,6 +520,38 @@ static function get_root_path($dir) {
         return null;
     }
 
+    /*
+     * get_base_url
+     *
+     * Get base url osTicket is installed on
+     * It Should match help desk url.
+     *
+     */
+    static function get_base_url() {
+        return sprintf('http%s://%s',
+                osTicket::is_https() ? 's' : '',
+                $_SERVER['HTTP_HOST'] . ROOT_PATH);
+    }
+
+    /*
+     * get_client_port
+     *
+     * Get client PORT from "Http_X-Forwarded-PORT" if we have trusted
+     * proxies set.
+     * FIXME: Follow trusted proxies chain
+     *
+     */
+    static function get_client_port($header='HTTP_X_FORWARDED_PORT') {
+        $port = $_SERVER['SERVER_PORT'];
+        // We're just making sure we have Trusted Proxies
+        // FIXME: Validate
+        $proxies = self::getTrustedProxies();
+        if (isset($_SERVER[$header]) &&  $proxies)
+            $port = $_SERVER[$header];
+
+        return $port;
+    }
+
     /*
      * get_client_ip
      *

diff --git a/include/client/tickets.inc.php b/include/client/tickets.inc.php
@@ -171,7 +171,7 @@
 
 
 <h1 style="margin:10px 0">
-    <a href="<?php echo Format::htmlchars($_SERVER['REQUEST_URI']); ?>"
+    <a href="<?php echo Html::refresh_url(); ?>"
         ><i class="refresh icon-refresh"></i>
     <?php echo __('Tickets'); ?>
     </a>

diff --git a/include/staff/tasks.inc.php b/include/staff/tasks.inc.php
@@ -5,17 +5,9 @@
 // Make sure the cdata materialized view is available
 TaskForm::ensureDynamicDataView();
 
-// Figure out REFRESH url — which might not be accurate after posting a
-// response
-list($path,) = explode('?', $_SERVER['REQUEST_URI'], 2);
-$args = array();
-parse_str($_SERVER['QUERY_STRING'], $args);
-
-// Remove commands from query
-unset($args['id']);
-unset($args['a']);
-
-$refresh_url = htmlspecialchars($path) . '?' . http_build_query($args);
+// Remove some variables from query string.
+$qsFilter = ['id', 'a'];
+$refresh_url = Http::refresh_url($qsFilter);
 
 $sort_options = array(
     'updated' =>            __('Most Recently Updated'),

diff --git a/include/staff/templates/queue-tickets.tmpl.php b/include/staff/templates/queue-tickets.tmpl.php
@@ -26,15 +26,11 @@
 
 // Figure out REFRESH url — which might not be accurate after posting a
 // response
-list($path,) = explode('?', $_SERVER['REQUEST_URI'], 2);
-$args = array();
-parse_str($_SERVER['QUERY_STRING'], $args);
-
-// Remove commands from query
-unset($args['id']);
-if (isset($args['a']) && ($args['a'] !== 'search')) unset($args['a']);
-
-$refresh_url = $path . '?' . http_build_query($args);
+// Remove some variables from query string.
+$qsFilter = ['id'];
+if (isset($_REQUEST['a']) && ($_REQUEST['a'] !== 'search'))
+    $qsFilter[] = 'a';
+$refresh_url = Http::refresh_url($qsFilter);
 
 // Establish the selected or default sorting mechanism
 if (isset($_GET['sort']) && is_numeric($_GET['sort'])) {

diff --git a/main.inc.php b/main.inc.php
@@ -27,8 +27,9 @@
 Bootstrap::loadCode();
 Bootstrap::connect();
 
-#Global override
+# Global Override when behind a proxy
 $_SERVER['REMOTE_ADDR'] = osTicket::get_client_ip();
+$_SERVER['SERVER_PORT'] = osTicket::get_client_port();
 
 if(!($ost=osTicket::start()) || !($cfg = $ost->getConfig()))
 Bootstrap::croak(__('Unable to load config info from DB.').' '.__('Get technical help!'));