security: Refresh URL

This initially targeted a particular vulnerability with the Advanced
Search Refresh URLs but subsequently evolved into unifying all Refresh
URLs. We decided to create methods that properly encode URLs to prevent
code execution and to ensure they are all uniformly created. This adds
such capabilities and updates current Refresh URL generation to the new
methods. This also adds a method `Format::http_query_string()` that will
filter the query string and remove any unwanted params. Lastly, this
removes the old and tired `Misc::currentURL()` in favor of the new
"hip" (is that what the kids say these days?) method `Http::url()`.
This commit is contained in:
protich
2022-10-26 16:28:38 -05:00
committed by JediKev
parent 75e111cecc
commit 3702a4fb3f
10 changed files with 96 additions and 48 deletions

View File

@@ -375,7 +375,7 @@ if (!defined('ROOT_PATH') && ($rp = osTicket::get_root_path(dirname(__file__))))
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);

View File

@@ -46,8 +46,7 @@ if ($cfg->isAuthRequiredForFiles()
// 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';

View File

@@ -407,6 +407,14 @@ class Format {
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);
}

View File

@@ -151,5 +151,48 @@ class Http {
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;
}
}
?>

View File

@@ -226,29 +226,6 @@ class Misc {
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;

View File

@@ -520,6 +520,38 @@ class osTicket {
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
*

View File

@@ -171,7 +171,7 @@ foreach (Topic::getHelpTopics(true) as $id=>$name) {
<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>

View File

@@ -5,17 +5,9 @@ $date_header = $date_col = false;
// 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'),

View File

@@ -26,15 +26,11 @@ $columns = $queue->getColumns();
// 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'])) {

View File

@@ -27,8 +27,9 @@ Bootstrap::i18n_prep();
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!'));