Skip to content

Commit

Permalink
security: Username XSS
Browse files Browse the repository at this point in the history
This mitigates a vulnerability reported by legoclones on
[huntr.dev](https://huntr.dev/) where XSS can be stored/executed via a
User's `username`. If an Agent is redirected to the User Lookup AJAX
response directly it will execute the stored XSS. This adds
`Format::sanitize()` to the `username` before saving to the database so any
potential XSS is sanitized. Additionally, this adds `Format::htmlchars()` to
the `username` before displaying in the UI to help prevent code execution.
  • Loading branch information
JediKev committed Nov 8, 2022
1 parent dc0507e commit 5213ff1
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 8 deletions.
4 changes: 2 additions & 2 deletions include/ajax.users.php
Expand Up @@ -34,7 +34,7 @@ function search($type = null, $fulltext=false) {
if (!$_REQUEST['q'])
return $this->json_encode($matches);

$q = $_REQUEST['q'];
$q = Format::sanitize($_REQUEST['q']);
$limit = isset($_REQUEST['limit']) ? (int) $_REQUEST['limit']:25;
$users=array();
$emails=array();
Expand Down Expand Up @@ -107,7 +107,7 @@ function search($type = null, $fulltext=false) {
}
$name = Format::htmlchars(new UsersName($name));
$matches[] = array('email'=>$email, 'name'=>$name, 'info'=>"$email - $name",
"id" => $id, "/bin/true" => $_REQUEST['q']);
"id" => $id, "/bin/true" => $q);
}
usort($matches, function($a, $b) { return strcmp($a['name'], $b['name']); });
}
Expand Down
2 changes: 1 addition & 1 deletion include/class.client.php
Expand Up @@ -484,7 +484,7 @@ function update($vars, &$errors) {
if ($vars['backend']) {
$this->set('backend', $vars['backend']);
if ($vars['username'])
$this->set('username', $vars['username']);
$this->set('username', Format::sanitize($vars['username']));
}

if ($vars['passwd1']) {
Expand Down
4 changes: 2 additions & 2 deletions include/class.user.php
Expand Up @@ -1320,7 +1320,7 @@ function update($vars, &$errors) {
}

$this->set('timezone', $vars['timezone']);
$this->set('username', $vars['username']);
$this->set('username', Format::sanitize($vars['username']));

if ($vars['passwd1']) {
$this->setPassword($vars['passwd1']);
Expand Down Expand Up @@ -1398,7 +1398,7 @@ static function register($user, $vars, &$errors) {
));

if ($vars['username'] && strcasecmp($vars['username'], $user->getEmail()))
$account->set('username', $vars['username']);
$account->set('username', Format::sanitize($vars['username']));

if ($vars['passwd1'] && !$vars['sendemail']) {
$account->set('passwd', Passwd::hash($vars['passwd1']));
Expand Down
2 changes: 1 addition & 1 deletion include/staff/templates/user-account.tmpl.php
Expand Up @@ -90,7 +90,7 @@
<?php echo __('Username'); ?>:
</td>
<td>
<input type="text" size="35" name="username" value="<?php echo $info['username']; ?>" autocomplete="new-password">
<input type="text" size="35" name="username" value="<?php echo Format::htmlchars($info['username']); ?>" autocomplete="new-password">
<i class="help-tip icon-question-sign" data-title="<?php
echo __("Login via email"); ?>"
data-content="<?php echo sprintf('%s: %s',
Expand Down
4 changes: 2 additions & 2 deletions include/staff/templates/user-register.tmpl.php
Expand Up @@ -68,8 +68,8 @@
<?php echo __('Username'); ?>:
</td>
<td>
<input type="text" size="35" name="username" value="<?php echo $info['username'] ?: $user->getEmail(); ?>">
&nbsp;<span class="error">&nbsp;<?php echo $errors['username']; ?></span>
<input type="text" size="35" name="username" value="<?php echo $info['username'] ? Format::htmlchars($info['username']) : $user->getEmail(); ?>">
&nbsp;<span class="error">&nbsp;<?php echo Format::htmlchars($errors['username']); ?></span>
</td>
</tr>
</tbody>
Expand Down

0 comments on commit 5213ff1

Please sign in to comment.