From 17d5dbf51db958944a8d02f364ba397677598657 Mon Sep 17 00:00:00 2001
From: Jared Hancock <gravydish@gmail.com>
Date: Fri, 18 Jul 2014 17:05:16 -0500
Subject: [PATCH] Fix potential XSS vulnerability on user's name
Add other locations as well a failsafe for the htmlentities() call
---
include/ajax.tickets.php | 8 ++++----
include/ajax.users.php | 6 +++---
include/class.format.php | 12 +++++++++---
include/class.organization.php | 6 +++++-
.../staff/templates/collaborators-preview.tmpl.php | 2 +-
include/staff/templates/collaborators.tmpl.php | 2 +-
include/staff/templates/user-account.tmpl.php | 2 +-
include/staff/templates/user-register.tmpl.php | 2 +-
include/staff/ticket-open.inc.php | 2 +-
include/staff/user-view.inc.php | 4 ++--
login.php | 2 +-
11 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/include/ajax.tickets.php b/include/ajax.tickets.php
index 5722a76a7..ccfb4b30d 100644
--- a/include/ajax.tickets.php
+++ b/include/ajax.tickets.php
@@ -390,15 +390,15 @@ class TicketsAjaxAPI extends AjaxController {
if ($user) {
if ($user->getId() == $ticket->getOwnerId())
$errors['err'] = sprintf('Ticket owner, %s, is a collaborator by default!',
- $user->getName());
+ Format::htmlchars($user->getName()));
elseif (($c=$ticket->addCollaborator($user,
array('isactive'=>1), $errors))) {
$note = Format::htmlchars(sprintf('%s <%s> added as a collaborator',
- $c->getName(), $c->getEmail()));
+ Format::htmlchars($c->getName()), $c->getEmail()));
$ticket->logNote('New Collaborator Added', $note,
$thisstaff, false);
$info = array('msg' => sprintf('%s added as a collaborator',
- $c->getName()));
+ Format::htmlchars($c->getName())));
return self::_collaborators($ticket, $info);
}
}
@@ -427,7 +427,7 @@ class TicketsAjaxAPI extends AjaxController {
return self::_collaborator($c ,$user->getForms($_POST), $errors);
$info = array('msg' => sprintf('%s updated successfully',
- $c->getName()));
+ Format::htmlchars($c->getName())));
return self::_collaborators($ticket, $info);
}
diff --git a/include/ajax.users.php b/include/ajax.users.php
index 8e31f3922..103999b64 100644
--- a/include/ajax.users.php
+++ b/include/ajax.users.php
@@ -113,7 +113,7 @@ class UsersAjaxAPI extends AjaxController {
Http::response(404, 'Unknown user');
$info = array(
- 'title' => sprintf('Update %s', $user->getName())
+ 'title' => sprintf('Update %s', Format::htmlchars($user->getName()))
);
$forms = $user->getForms();
@@ -357,7 +357,7 @@ class UsersAjaxAPI extends AjaxController {
Http::response(404, 'Unknown user');
$info = array();
- $info['title'] = 'Organization for '.$user->getName();
+ $info['title'] = 'Organization for '.Format::htmlchars($user->getName());
$info['action'] = '#users/'.$user->getId().'/org';
$info['onselect'] = 'ajax.php/users/'.$user->getId().'/org';
@@ -379,7 +379,7 @@ class UsersAjaxAPI extends AjaxController {
} elseif ($orgId)
$org = Organization::lookup($orgId);
elseif ($org = $user->getOrganization()) {
- $info['title'] = sprintf('%s — %s', $user->getName(), 'Organization');
+ $info['title'] = sprintf('%s — %s', Format::htmlchars($user->getName()), 'Organization');
$info['action'] = $info['onselect'] = '';
$tmpl = 'org.tmpl.php';
}
diff --git a/include/class.format.php b/include/class.format.php
index 654a9c4e2..000d771bb 100644
--- a/include/class.format.php
+++ b/include/class.format.php
@@ -269,13 +269,19 @@ class Format {
}
function htmlencode($var) {
+
+ if (is_array($var))
+ return array_map(array('Format', 'htmlencode'), $var);
+
$flags = ENT_COMPAT;
if (phpversion() >= '5.4.0')
$flags |= ENT_HTML401;
- return is_array($var)
- ? array_map(array('Format', 'htmlencode'), $var)
- : htmlentities( (string) $var, $flags, 'UTF-8', false);
+ try {
+ return htmlentities( (string) $var, $flags, 'UTF-8', false);
+ } catch(Exception $e) {
+ return $var;
+ }
}
function htmldecode($var) {
diff --git a/include/class.organization.php b/include/class.organization.php
index 3a8a11b02..462b25df2 100644
--- a/include/class.organization.php
+++ b/include/class.organization.php
@@ -152,7 +152,11 @@ class Organization extends OrganizationModel {
}
function getInfo() {
- $base = $this->ht;
+
+ $base = array_filter($this->ht,
+ function ($e) { return !is_object($e); }
+ );
+
foreach (array(
'collab-all-flag' => Organization::COLLAB_ALL_MEMBERS,
'collab-pc-flag' => Organization::COLLAB_PRIMARY_CONTACT,
diff --git a/include/staff/templates/collaborators-preview.tmpl.php b/include/staff/templates/collaborators-preview.tmpl.php
index bd9a3b6d0..db8626ab2 100644
--- a/include/staff/templates/collaborators-preview.tmpl.php
+++ b/include/staff/templates/collaborators-preview.tmpl.php
@@ -8,7 +8,7 @@ if (($users=$ticket->getCollaborators())) {?>
echo sprintf('<tr><td %s><i class="icon-%s"></i> %s <em><%s></em></td></tr>',
($user->isActive()? '' : 'class="faded"'),
($user->isActive()? 'comments' : 'comment-alt'),
- $user->getName(),
+ Format::htmlchars($user->getName()),
$user->getEmail());
}
} else {
diff --git a/include/staff/templates/collaborators.tmpl.php b/include/staff/templates/collaborators.tmpl.php
index 2c0de2a2e..d2bbc871a 100644
--- a/include/staff/templates/collaborators.tmpl.php
+++ b/include/staff/templates/collaborators.tmpl.php
@@ -27,7 +27,7 @@ if(($users=$ticket->getCollaborators())) {?>
$user->getId(),
$checked,
$user->getId(),
- $user->getName(),
+ Format::htmlchars($user->getName()),
$user->getEmail(),
$user->getId(),
$user->getId());
diff --git a/include/staff/templates/user-account.tmpl.php b/include/staff/templates/user-account.tmpl.php
index 8304d8afa..1ed07b9ea 100644
--- a/include/staff/templates/user-account.tmpl.php
+++ b/include/staff/templates/user-account.tmpl.php
@@ -39,7 +39,7 @@ if ($info['error']) {
<td width="180">
Name:
</td>
- <td> <?php echo $user->getName(); ?> </td>
+ <td> <?php echo Format::htmlchars($user->getName()); ?> </td>
</tr>
<tr>
<td width="180">
diff --git a/include/staff/templates/user-register.tmpl.php b/include/staff/templates/user-register.tmpl.php
index 99fbc3f34..f0c01449c 100644
--- a/include/staff/templates/user-register.tmpl.php
+++ b/include/staff/templates/user-register.tmpl.php
@@ -28,7 +28,7 @@ if ($info['error']) {
} ?>
<div><p id="msg_info"><i class="icon-info-sign"></i> Complete the form
below to create a user account for <b><?php echo
-$user->getName()->getOriginal(); ?></b>.</p></div>
+Format::htmlchars($user->getName()->getOriginal()); ?></b>.</p></div>
<div id="user-registration" style="display:block; margin:5px;">
<form method="post" class="user"
action="#users/<?php echo $user->getId(); ?>/register">
diff --git a/include/staff/ticket-open.inc.php b/include/staff/ticket-open.inc.php
index bfd0571bd..00055c072 100644
--- a/include/staff/ticket-open.inc.php
+++ b/include/staff/ticket-open.inc.php
@@ -53,7 +53,7 @@ if ($info['topicId'] && ($topic=Topic::lookup($info['topicId']))) {
});
return false;
"><i class="icon-user"></i>
- <span id="user-name"><?php echo $user->getName(); ?></span>
+ <span id="user-name"><?php echo Format::htmlchars($user->getName()); ?></span>
<<span id="user-email"><?php echo $user->getEmail(); ?></span>>
</a>
<a class="action-button" style="float:none;overflow:inherit" href="#"
diff --git a/include/staff/user-view.inc.php b/include/staff/user-view.inc.php
index 6a29a3a0e..65b47c7d1 100644
--- a/include/staff/user-view.inc.php
+++ b/include/staff/user-view.inc.php
@@ -10,7 +10,7 @@ $org = $user->getOrganization();
<tr>
<td width="50%" class="has_bottom_border">
<h2><a href="users.php?id=<?php echo $user->getId(); ?>"
- title="Reload"><i class="icon-refresh"></i> <?php echo $user->getName(); ?></a></h2>
+ title="Reload"><i class="icon-refresh"></i> <?php echo Format::htmlchars($user->getName()); ?></a></h2>
</td>
<td width="50%" class="right_align has_bottom_border">
<span class="action-button" data-dropdown="#action-dropdown-more">
@@ -69,7 +69,7 @@ $org = $user->getOrganization();
<td><b><a href="#users/<?php echo $user->getId();
?>/edit" class="user-action"><i
class="icon-edit"></i> <?php echo
- $user->getName()->getOriginal();
+ Format::htmlchars($user->getName()->getOriginal());
?></a></td>
</tr>
<tr>
diff --git a/login.php b/login.php
index d8c4d8101..11c09ec34 100644
--- a/login.php
+++ b/login.php
@@ -76,7 +76,7 @@ elseif ($_POST && isset($_POST['lticket'])) {
// force attempts (which doesn't buy much since the link is emailed)
$user->sendAccessLink();
$msg = sprintf("%s - access link sent to your email!",
- $user->getName()->getFirst());
+ Format::htmlchars($user->getName()->getFirst()));
$_POST = null;
} elseif(!$errors['err']) {
$errors['err'] = 'Invalid email or ticket number - try again!';
--
GitLab