From eab6747e9518d715d4301b8cd159f2523685e603 Mon Sep 17 00:00:00 2001
From: JediKev <kevin@enhancesoft.com>
Date: Wed, 24 Apr 2019 11:29:09 -0500
Subject: [PATCH] xss: XSS To LFI Vulnerability
This addresses a vulnerability found by [AkkuS CW](https://pentest.com.tr)
where a simple XSS attempt can lead to an LFI (Local File Inclusion) attack.
The issue stems from the system returning the unformatted file contents in
an error message when uploading a CSV to the User Importer. This formats the
contents before uploading so that if the contents are returned in an error
message they will not be executed by the browser which therefore prevents
XSS attempts and the possibility of an LFI attack. This also formats all the
user-created data sent to ImportError to prevent the same issue.
---
include/class.import.php | 8 ++++----
include/class.staff.php | 4 ++--
include/class.user.php | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/class.import.php b/include/class.import.php
index dc9a027c1..7e887e769 100644
--- a/include/class.import.php
+++ b/include/class.import.php
@@ -39,7 +39,7 @@ class CsvImporter {
rewind($this->stream);
}
else {
- throw new ImportError(__('Unable to parse submitted csv: ').print_r($stream, true));
+ throw new ImportError(__('Unable to parse submitted csv: ').print_r(Format::htmlchars($stream), true));
}
}
@@ -59,7 +59,7 @@ class CsvImporter {
throw new ImportError(__('Whoops. Perhaps you meant to send some CSV records'));
$headers = array();
- foreach ($data as $h) {
+ foreach (Format::htmlchars($data) as $h) {
$h = trim($h);
$found = false;
foreach ($all_fields as $f) {
@@ -68,7 +68,7 @@ class CsvImporter {
$found = true;
if (!$f->get('name'))
throw new ImportError(sprintf(__(
- '%s: Field must have `variable` set to be imported'), $h));
+ '%s: Field must have `variable` set to be imported'), Format::htmlchars($h)));
$headers[$f->get('name')] = $f->get('label');
break;
}
@@ -85,7 +85,7 @@ class CsvImporter {
}
else {
throw new ImportError(sprintf(
- __('%s: Unable to map header to the object field'), $h));
+ __('%s: Unable to map header to the object field'), Format::htmlchars($h)));
}
}
}
diff --git a/include/class.staff.php b/include/class.staff.php
index 941f8baba..6d0a6e92c 100644
--- a/include/class.staff.php
+++ b/include/class.staff.php
@@ -946,8 +946,8 @@ implements AuthenticatedUser, EmailContact, TemplateVariable {
}
else {
throw new ImportError(sprintf(__('Unable to import (%s): %s'),
- $data['username'],
- print_r($errors, true)
+ Format::htmlchars($data['username']),
+ print_r(Format::htmlchars($errors), true)
));
}
$imported++;
diff --git a/include/class.user.php b/include/class.user.php
index e16259ce7..f991e10ad 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -456,7 +456,7 @@ implements TemplateVariable {
throw new ImportError('Both `name` and `email` fields are required');
if (!($user = static::fromVars($data, true, true)))
throw new ImportError(sprintf(__('Unable to import user: %s'),
- print_r($data, true)));
+ print_r(Format::htmlchars($data), true)));
$imported++;
}
db_autocommit(true);
--
GitLab