From eab6747e9518d715d4301b8cd159f2523685e603 Mon Sep 17 00:00:00 2001 From: JediKev 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 dc9a027c1d..7e887e7690 100644 --- a/include/class.import.php +++ b/include/class.import.php @@ -39,7 +39,7 @@ function __construct($stream) { 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 @@ function importCsv($all_fields=array(), $defaults=array()) { 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 @@ function importCsv($all_fields=array(), $defaults=array()) { $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 @@ function importCsv($all_fields=array(), $defaults=array()) { } 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 941f8baba2..6d0a6e92cb 100644 --- a/include/class.staff.php +++ b/include/class.staff.php @@ -946,8 +946,8 @@ static function importCsv($stream, $defaults=array(), $callback=false) { } 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 e16259ce7e..f991e10ad5 100644 --- a/include/class.user.php +++ b/include/class.user.php @@ -456,7 +456,7 @@ static function importCsv($stream, $defaults=array()) { 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);