From 49008238c7bf9a23801d86a693eb7e70058199ed Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Thu, 4 Nov 2021 15:45:14 +0800 Subject: [PATCH] MDL-72096 core: Add safe ORDER BY helpers for db sorting from user input The new get_safe_orderby() and get_safe_orderby_multiple() methods provide a centralised safe way for user submitted sorting values to be incorporated into SQL ORDER BY. They do this by removing the need for user submitted data to pass in any SQL and not allowing arbitrary column values, instead using string keys which map to a predefined list of allowed sortable columns. --- lib/datalib.php | 92 ++++++++++++++++++++++++++++++ lib/tests/datalib_test.php | 114 +++++++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+) diff --git a/lib/datalib.php b/lib/datalib.php index 81adff36a5e..4b3bb7e6020 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -1852,3 +1852,95 @@ function get_max_courses_in_category() { return $CFG->maxcoursesincategory; } } + +/** + * Prepare a safe ORDER BY statement from user interactable requests. + * + * This allows safe user specified sorting (ORDER BY), by abstracting the SQL from the value being requested by the user. + * A standard string (and optional direction) can be specified, which will be mapped to a predefined allow list of SQL ordering. + * The mapping can optionally include a 'default', which will be used if the key provided is invalid. + * + * Example usage: + * -If $orderbymap = [ + * 'courseid' => 'c.id', + * 'somecustomvalue'=> 'c.startdate, c.shortname', + * 'default' => 'c.fullname', + * ] + * -A value from the above array's keys can be passed in by some user interaction (eg web service), along with an optional direction. + * -get_safe_orderby($orderbymap, 'courseid', 'DESC') would return: ORDER BY c.id DESC + * -get_safe_orderby($orderbymap, 'somecustomvalue') would return: ORDER BY c.startdate, c.shortname + * -get_safe_orderby($orderbymap, 'invalidblah', 'DESC') would return: ORDER BY c.fullname DESC + * -If no default key was specified in $orderbymap, the invalidblah example above would return empty string. + * + * @param array $orderbymap An array in the format [keystring => sqlstring]. A default fallback can be set with the key 'default'. + * @param string $orderbykey A string to be mapped to a key in $orderbymap. + * @param string $direction Optional ORDER BY direction (ASC/DESC). + * @param bool $useprefix Whether ORDER BY is prefixed to the output (true by default). This should not be modified in most cases. + * It is included to enable get_safe_orderby_multiple() to use this function multiple times. + * @return string The ORDER BY statement, or empty string if $orderbykey is invalid and no default is mapped. + */ +function get_safe_orderby(array $orderbymap, string $orderbykey, string $direction = '', bool $useprefix = true): string { + $orderby = $useprefix ? ' ORDER BY ': ''; + $output = ''; + + // Only include an order direction if ASC/DESC is explicitly specified. + if (!in_array($direction, ['ASC', 'DESC'], true)) { + $direction = ''; + } else { + $direction = " {$direction}"; + } + + // Prepare the statement if the key maps to a defined sort parameter. + if (isset($orderbymap[$orderbykey])) { + $output = "{$orderby}{$orderbymap[$orderbykey]}{$direction}"; + } else if (array_key_exists('default', $orderbymap)) { + // Fall back to use the default if one is specified. + $output = "{$orderby}{$orderbymap['default']}{$direction}"; + } + + return $output; +} + +/** + * Prepare a safe ORDER BY statement from user interactable requests using multiple values. + * + * This allows safe user specified sorting (ORDER BY) similar to get_safe_orderby(), but allows multiple keys and directions to be passed in. + * This is useful in cases where many combinations of columns are needed and/or each item requires a direction (ASC/DESC) to be specified. + * The mapping can optionally include a 'default', which will be used if the key provided is invalid. + * + * Example usage: + * -If $orderbymap = [ + * 'courseid' => 'c.id', + * 'fullname'=> 'c.fullname', + * 'default' => 'c.startdate', + * ] + * -A value from the above array's keys can be passed in by some user interaction (eg web service), along with an optional direction. + * -get_safe_orderby($orderbymap, ['courseid', 'fullname'], ['DESC', 'ASC']) would return: ORDER BY c.id DESC, c.fullname ASC + * -get_safe_orderby($orderbymap, ['courseid', 'invalidblah'], ['aaa', 'DESC']) would return: ORDER BY c.id, c.startdate DESC + * -If no default key was specified in $orderbymap, the invalidblah example above would return: ORDER BY c.id + * + * @param array $orderbymap An array in the format [keystring => sqlstring]. A default fallback can be set with the key 'default'. + * @param array $orderbykeys An array of strings to be mapped to keys in $orderbymap. + * @param array $directions Optional array of ORDER BY direction (ASC/DESC). The array keys should match array keys in $orderbykeys. + * @return string The ORDER BY statement, or empty string if $orderbykeys contains no valid items and no default is mapped. + */ +function get_safe_orderby_multiple(array $orderbymap, array $orderbykeys, array $directions = []): string { + $output = ''; + + // Check each key for a valid mapping and add to the ORDER BY statement (invalid entries will be empty strings). + foreach ($orderbykeys as $index => $orderbykey) { + $direction = $directions[$index] ?? ''; + $safeorderby = get_safe_orderby($orderbymap, $orderbykey, $direction, false); + + if (!empty($safeorderby)) { + $output .= ", {$safeorderby}"; + } + } + + // Prefix with ORDER BY if any valid ordering is specified (and remove comma from the start). + if (!empty($output)) { + $output = ' ORDER BY' . ltrim($output, ','); + } + + return $output; +} diff --git a/lib/tests/datalib_test.php b/lib/tests/datalib_test.php index b81ddefdab6..e352bd45e3e 100644 --- a/lib/tests/datalib_test.php +++ b/lib/tests/datalib_test.php @@ -894,4 +894,118 @@ class core_datalib_testcase extends advanced_testcase { $results = array_diff_key($results, $existingids); $this->assertEquals([$userids[1], $userids[3]], array_keys($results)); } + + /** + * Tests the get_safe_orderby function. + */ + public function test_get_safe_orderby(): void { + $orderbymap = [ + 'courseid' => 'c.id', + 'somecustomvalue'=> 'c.startdate, c.shortname', + 'default' => 'c.fullname', + ]; + $orderbymapnodefault = [ + 'courseid' => 'c.id', + 'somecustomvalue'=> 'c.startdate, c.shortname', + ]; + + // Test valid option, no direction specified. + $actual = get_safe_orderby($orderbymap, 'somecustomvalue'); + $this->assertEquals(' ORDER BY c.startdate, c.shortname', $actual); + + // Test valid option, valid direction specified. + $actual = get_safe_orderby($orderbymap, 'courseid', 'DESC'); + $this->assertEquals(' ORDER BY c.id DESC', $actual); + + // Test valid option, invalid direction specified. + $actual = get_safe_orderby($orderbymap, 'courseid', 'BOOP'); + $this->assertEquals(' ORDER BY c.id', $actual); + + // Test invalid option default fallback, with valid direction. + $actual = get_safe_orderby($orderbymap, 'thisdoesnotexist', 'ASC'); + $this->assertEquals(' ORDER BY c.fullname ASC', $actual); + + // Test invalid option default fallback, with invalid direction. + $actual = get_safe_orderby($orderbymap, 'thisdoesnotexist', 'BOOP'); + $this->assertEquals(' ORDER BY c.fullname', $actual); + + // Test invalid option without default, with valid direction. + $actual = get_safe_orderby($orderbymapnodefault, 'thisdoesnotexist', 'ASC'); + $this->assertEquals('', $actual); + + // Test invalid option without default, with invalid direction. + $actual = get_safe_orderby($orderbymapnodefault, 'thisdoesnotexist', 'NOPE'); + $this->assertEquals('', $actual); + + // Test noprefix parameter returns the statement without "ORDER BY". + $actual = get_safe_orderby($orderbymap, 'courseid', 'DESC', false); + $this->assertEquals('c.id DESC', $actual); + } + + /** + * Tests the get_safe_orderby_multiple function. + */ + public function test_get_safe_orderby_multiple(): void { + $orderbymap = [ + 'courseid' => 'c.id', + 'firstname'=> 'u.firstname', + 'default' => 'c.startdate', + ]; + $orderbymapnodefault = [ + 'courseid' => 'c.id', + 'firstname'=> 'u.firstname', + ]; + + // Test valid options, no directions specified. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'firstname']); + $this->assertEquals(' ORDER BY c.id, u.firstname', $actual); + + // Test valid options, some direction specified. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'firstname'], ['DESC']); + $this->assertEquals(' ORDER BY c.id DESC, u.firstname', $actual); + + // Test valid options, all directions specified. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'firstname'], ['ASC', 'DESC']); + $this->assertEquals(' ORDER BY c.id ASC, u.firstname DESC', $actual); + + // Test valid options, valid and invalid directions specified. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'firstname'], ['BOOP', 'DESC']); + $this->assertEquals(' ORDER BY c.id, u.firstname DESC', $actual); + + // Test valid options, all invalid directions specified. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'firstname'], ['BOOP', 'SNOOT']); + $this->assertEquals(' ORDER BY c.id, u.firstname', $actual); + + // Test valid and invalid option default fallback, with valid direction. + $actual = get_safe_orderby_multiple($orderbymap, ['thisdoesnotexist', 'courseid'], ['ASC', 'DESC']); + $this->assertEquals(' ORDER BY c.startdate ASC, c.id DESC', $actual); + + // Test valid and invalid option default fallback, with invalid direction. + $actual = get_safe_orderby_multiple($orderbymap, ['courseid', 'thisdoesnotexist'], ['BOOP', 'SNOOT']); + $this->assertEquals(' ORDER BY c.id, c.startdate', $actual); + + // Test valid and invalid option without default, with valid direction. + $actual = get_safe_orderby_multiple($orderbymapnodefault, ['thisdoesnotexist', 'courseid'], ['ASC', 'DESC']); + $this->assertEquals(' ORDER BY c.id DESC', $actual); + + // Test valid and invalid option without default, with invalid direction. + $actual = get_safe_orderby_multiple($orderbymapnodefault, ['thisdoesnotexist', 'courseid'], ['BOOP', 'SNOOT']); + $this->assertEquals(' ORDER BY c.id', $actual); + + // Test invalid option only without default, with valid direction. + $actual = get_safe_orderby_multiple($orderbymapnodefault, ['thisdoesnotexist'], ['ASC']); + $this->assertEquals('', $actual); + + // Test invalid option only without default, with invalid direction. + $actual = $actual = get_safe_orderby_multiple($orderbymapnodefault, ['thisdoesnotexist'], ['BOOP']); + $this->assertEquals('', $actual); + + // Test single valid option, direction specified. + $actual = get_safe_orderby_multiple($orderbymap, ['firstname'], ['ASC']); + $this->assertEquals(' ORDER BY u.firstname ASC', $actual); + + // Test single valid option, direction not specified. + $actual = get_safe_orderby_multiple($orderbymap, ['firstname']); + $this->assertEquals(' ORDER BY u.firstname', $actual); + } } -- 2.17.1