Skip to content

Commit

Permalink
Add input validation to report generator controller endpoints.
Browse files Browse the repository at this point in the history
This pull request adds input validation filters to the user-supplied
untrusted data provided to the report generator controller endpoints.

The modifications do not change any of the data serialization or api of
the endpoints.

The user-supplied report text (title, header, footer etc.) is modified
to filter out data that does not have a valid character encoding. This
mitigates a bug in the report generator if you tried to create a report
with a filename containing an invalid character such as 😊 then the UI would
show an error message:

```
There was a problem creating / updating this report.
(SQLSTATE[HY000]: General error: 1267 Illegal mix of collations (latin1_swedish_ci,IMPLICIT) and (utf8_general_ci,COERCIBLE) for operation 'like')
```

the new behaviour is that the report will save ok but any invalid
characters will be converted to '?'. A complete fix for this bug is
beyond the scope of this pull request.

There are several complex regular expressions used to validate the data
this is necessary due to the non-standard serialization used in the
report generator.  Unit tests have been added to confirm that the more
complex regular expressions used do pass through permissable data.
  • Loading branch information
jpwhite4 committed Jan 23, 2020
1 parent 6c622a6 commit 4a4fbae
Show file tree
Hide file tree
Showing 6 changed files with 298 additions and 63 deletions.
36 changes: 36 additions & 0 deletions classes/DataWarehouse/Access/ReportGenerator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace DataWarehouse\Access;

class ReportGenerator extends Common
{
/* Properties of reports. See also the enum columns in the moddb.Reports
* table */
const REPORT_ID_REGEX = '/^[0-9]+-[0-9\.]+$/';
const REPORT_DATE_REGEX = '/^[0-9]{4}(-[0-9]{2}){2}$/';
const REPORT_FORMATS_REGEX = '/^doc|pdf$/';
const REPORT_FONT_REGEX = '/^Arial$/';
const REPORT_SCHEDULE_REGEX = '/^Once|Daily|Weekly|Monthly|Quarterly|Semi-annually|Annually$/';
const REPORT_DELIVERY_REGEX = '/^E-Mail$/';

/* Patterns related to report charts */
const REPORT_CHART_TYPE_REGEX = '/^chart_pool|volatile|report|cached$/';
const REPORT_CHART_REF_REGEX = '/^[0-9]+(-[0-9]+)?;[0-9]+$/';
const REPORT_CHART_DID_REGEX = '/^_d[0-9]+$/';

/* the save_report controller use a custom data serialization for charts
* that have been modified from the original report */
const CHART_CACHEREF_REGEX = '/^([0-9]{4}(-[0-9]{2}){2};){2}(?(?=xd_report_volatile_)xd_report_volatile_[0-9]+;[0-9]+(_d[0-9]+)?|[0-9]+-[0-9\.]+;[0-9]+)$/';

/* The report download endpoint retrieves the report data from a temporary
* directory that is created dynamically based on the report_id
*/
const REPORT_TMPDIR_REGEX = '/^[0-9]+-[0-9\.]+-[a-zA-Z0-9\.]+$/';

/*
* Character encoding used in the (user supplied) text contained in the
* report. This must be consistent with the character set used in the
* moddb.Report table.
*/
const REPORT_CHAR_ENCODING = 'ISO-8859-1';
}
24 changes: 20 additions & 4 deletions html/controllers/report_builder/download_report.php
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
<?php

use \DataWarehouse\Access\ReportGenerator;

$filters = array(
'format' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_FORMATS_REGEX)
),
'report_loc' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_TMPDIR_REGEX)
)
);

\xd_security\assertParametersSet(array(
'report_loc',
'format'
));

try {
if (XDReportManager::isValidFormat($_GET['format']) == false) {

$get = filter_input_array(INPUT_GET, $filters);

if (XDReportManager::isValidFormat($get['format']) == false) {
print "Invalid format specified";
exit;
}

$output_format = $_GET['format'];
$output_format = $get['format'];

$user = \xd_security\getLoggedInUser();

Expand All @@ -21,9 +37,9 @@

// Resolve absolute path to report document on backend

$report_id = preg_replace('/(.+)-(.+)-(.+)/', '$1-$2', $_GET['report_loc']);
$report_id = preg_replace('/(.+)-(.+)-(.+)/', '$1-$2', $get['report_loc']);

$working_directory = sys_get_temp_dir() . '/' . $_GET['report_loc'];
$working_directory = sys_get_temp_dir() . '/' . $get['report_loc'];

$report_file = $working_directory.'/'.$report_id.'.'.$output_format;

Expand Down
75 changes: 58 additions & 17 deletions html/controllers/report_builder/save_report.php
Original file line number Diff line number Diff line change
@@ -1,19 +1,54 @@
<?php

use \DataWarehouse\Access\ReportGenerator;

$filters = array(
'phase' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => '/^create|update$/')
),
'report_id' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_ID_REGEX)
),
'report_font' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_FONT_REGEX)
),
'report_format' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_FORMATS_REGEX . 'i')
),
'charts_per_page' => array(
'filter' => FILTER_VALIDATE_INT,
'options' => array('min_range' => 1)
),
'report_schedule' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_SCHEDULE_REGEX)
),
'report_delivery' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_DELIVERY_REGEX)
)
);

try {

$user = \xd_security\getLoggedInUser();
$rm = new XDReportManager($user);

$base_path = \xd_utilities\getConfiguration('reporting', 'base_path');

$post = filter_input_array(INPUT_POST, $filters);

$map = array();

// -----------------------------------

\xd_security\assertParameterSet('phase');

switch($_POST['phase']) {
switch($post['phase']) {

case 'create':

Expand All @@ -25,7 +60,7 @@

\xd_security\assertParameterSet('report_id');

$report_id = $_POST['report_id'];
$report_id = $post['report_id'];

// Cache the blobs so they can be re-introduced as necessary during the report update process
$rm->buildBlobMap($report_id, $map);
Expand All @@ -34,14 +69,14 @@

break;

}//switch($_POST['phase'])
}

// -----------------------------------
$report_name = $_POST['report_name'];
$report_title = $_POST['report_title'];
$report_header = $_POST['report_header'];
$report_footer = $_POST['report_footer'];

$report_name = mb_convert_encoding($_POST['report_name'], ReportGenerator::REPORT_CHAR_ENCODING, 'UTF-8');
$report_title = mb_convert_encoding($_POST['report_title'], ReportGenerator::REPORT_CHAR_ENCODING, 'UTF-8');
$report_header = mb_convert_encoding($_POST['report_header'], ReportGenerator::REPORT_CHAR_ENCODING, 'UTF-8');
$report_footer = mb_convert_encoding($_POST['report_footer'], ReportGenerator::REPORT_CHAR_ENCODING, 'UTF-8');

$rm->configureSelectedReport(

Expand All @@ -50,11 +85,11 @@
$report_title,
$report_header,
$report_footer,
$_POST['report_font'],
$_POST['report_format'],
$_POST['charts_per_page'],
$_POST['report_schedule'],
$_POST['report_delivery']
$post['report_font'],
$post['report_format'],
$post['charts_per_page'],
$post['report_schedule'],
$post['report_delivery']

);

Expand All @@ -68,7 +103,7 @@

// -----------------------------------

switch($_POST['phase']) {
switch($post['phase']) {

case 'create':

Expand All @@ -80,7 +115,7 @@
$rm->saveThisReport();
break;

}//switch($_POST['phase'])
}

// -----------------------------------

Expand All @@ -101,7 +136,13 @@

if (isset($_POST[$cache_ref_variable])) {

list($start_date, $end_date, $ref, $rank) = explode(';', $_POST[$cache_ref_variable]);
$cache_ref = filter_var(
$_POST[$cache_ref_variable],
FILTER_VALIDATE_REGEXP,
array('options' => array('regexp' => ReportGenerator::CHART_CACHEREF_REGEX))
);

list($start_date, $end_date, $ref, $rank) = explode(';', $cache_ref);

$location = sys_get_temp_dir() . "/{$ref}_{$rank}_{$start_date}_{$end_date}.png";

Expand Down Expand Up @@ -162,7 +203,7 @@
// -----------------------------------

$returnData['action'] = 'save_report';
$returnData['phase'] = $_POST['phase'];
$returnData['phase'] = $post['phase'];
$returnData['report_id'] = $report_id;
$returnData['success'] = true;
$returnData['status'] = 'success';
Expand Down
66 changes: 43 additions & 23 deletions html/controllers/report_builder/send_report.php
Original file line number Diff line number Diff line change
@@ -1,37 +1,57 @@
<?php

use \DataWarehouse\Access\ReportGenerator;

$filters = array(
'build_only' => array(
'filter' => FILTER_VALIDATE_BOOLEAN
),
'report_id' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_ID_REGEX)
),
'start_date' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_DATE_REGEX)
),
'end_date' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_DATE_REGEX)
),
'export_format' => array(
'filter' => FILTER_VALIDATE_REGEXP,
'options' => array('regexp' => ReportGenerator::REPORT_FORMATS_REGEX)
)
);

try {

$userdata = filter_input_array(INPUT_POST, $filters);

$user = \xd_security\getLoggedInUser();

$rm = new XDReportManager($user);

\xd_security\assertParametersSet(array(
'report_id',
'build_only'
));

$report_id = $_POST['report_id'];
$build_only = $_POST['build_only'];

// ==========================================================================

$export_format = XDReportManager::DEFAULT_FORMAT;

if (isset($_POST['export_format']) && (XDReportManager::isValidFormat($_POST['export_format']) == true)) {
$export_format = $_POST['export_format'];
}
$report_id = $userdata['report_id'];
if ($report_id === null) {
\xd_response\presentError('Invalid value specified for report_id');
}

if (isset($_POST['start_date']) && !empty($_POST['start_date']) && isset($_POST['end_date']) && !empty($_POST['end_date'])) {
$start_date = $_POST['start_date'];
$end_date = $_POST['end_date'];
} else {
$start_date = null;
$end_date = null;
$build_only = $userdata['build_only'];
if ($build_only === null) {
\xd_response\presentError('Invalid value specified for build_only');
}

$export_format = $userdata['export_format'];
if ($export_format === null) {
$export_format = XDReportManager::DEFAULT_FORMAT;
}

$start_date = $userdata['start_date'];
$end_date = $userdata['end_date'];

$returnData['action'] = 'send_report';
$returnData['build_only'] = ($build_only == "true");
$returnData['build_only'] = $build_only;

try {

Expand All @@ -40,7 +60,7 @@
$working_dir = $build_response['template_path'];
$report_filename = $build_response['report_file'];

if ($build_only == "true") {
if ($build_only) {

// Present enough information so that the download_report controller can serve up the file
// (and provide appropriate cleanup) afterwards.
Expand Down
Loading

0 comments on commit 4a4fbae

Please sign in to comment.