From 09d26bfec188c3b1ecd98f426aa14f8662ae8f45 Mon Sep 17 00:00:00 2001 From: Benn Oshrin Date: Tue, 10 Sep 2019 17:54:23 -0400 Subject: [PATCH] Improve validation of field names associated with SQL identifiers (CO-1767) --- app/src/Lib/Traits/ValidationTrait.php | 98 +++++++++++++++++++++++++ app/src/Locale/en_US/default.po | 13 ++-- app/src/Model/Table/AttributesTable.php | 5 +- app/src/Model/Table/MatchgridsTable.php | 4 +- 4 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 app/src/Lib/Traits/ValidationTrait.php diff --git a/app/src/Lib/Traits/ValidationTrait.php b/app/src/Lib/Traits/ValidationTrait.php new file mode 100644 index 000000000..5ff364e4f --- /dev/null +++ b/app/src/Lib/Traits/ValidationTrait.php @@ -0,0 +1,98 @@ +. Arguably, we should accept + // anything at all for input (and filter only on output), but this was agreed to + // as an extra "line of defense" against unsanitized HTML output, since there are + // currently no known cases where user-entered input should permit angle brackets. + +// XXX we previously supported 'filter'. 'flags', and 'invalidchars' as arguments, do we still need to? + + // What component are we? + $COmponent = __('product.code'); + + // Perform a basic string search. + + $invalid = "<>"; + + if(strlen($value) != strcspn($value, $invalid)) { + // Mismatch, implying bad input + return __($COmponent.'.er.input.invalid'); + } + + // We require at least one non-whitespace character (CO-1551) + if(!preg_match('/\S/', $value)) { + return __($COmponent.'.er.input.blank'); + } + + return true; + } + + /** + * Determine if a string submitted from a form is valid SQL identifier. + * + * @since COmanage Common v1.0.0 + * @param string $value Value to validate + * @param array $context Validation context + * @return mixed True if $value validates, or an error string otherwise + */ + + public function validateSqlIdentifier($value, array $context) { + // What component are we? + $COmponent = __('product.code'); + + // Valid (portable) SQL identifiers begin with a letter or underscore, and + // subsequent characters can also include digits. We'll be a little stricter + // than we need to be for now by only accepting A-Z, when in fact certain + // additional characters (like รก) are also acceptable. + + if(!preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $value)) { + return __($COmponent.'.er.input.invalid'); + } + + return true; + } +} \ No newline at end of file diff --git a/app/src/Locale/en_US/default.po b/app/src/Locale/en_US/default.po index ce84a19bd..7f92c2be8 100644 --- a/app/src/Locale/en_US/default.po +++ b/app/src/Locale/en_US/default.po @@ -191,6 +191,12 @@ msgstr "Cannot read file {0}" msgid "match.er.format" msgstr "Invalid format" +msgid "match.er.input.blank" +msgstr "Value cannot consist of only blank characters" + +msgid "match.er.input.invalid" +msgstr "Invalid character found" + msgid "match.er.mgid" msgstr "Could not find Matchgrid ID in request" @@ -233,9 +239,6 @@ msgstr "{0} does not have any valid permissions" msgid "match.er.search_type" msgstr "Unknown search type '{0}'" -msgid "match.er.val.alphanum" -msgstr "Provided value contains non-alphanumeric characters" - msgid "match.er.val.length" msgstr "Provided value exceeds maximum length of {0}" @@ -267,7 +270,7 @@ msgid "match.fd.AttributeMappings.value.desc" msgstr "Value used as a matching string to query key" msgid "match.fd.Attributes.name.desc" -msgstr "Value must be alphanumeric, as it will be used to construct the matchgrid column name" +msgstr "Value must be a valid SQL identifier, as it will be used to construct the matchgrid column name" msgid "match.fd.case_sensitive" msgstr "Case Sensitive" @@ -360,7 +363,7 @@ msgid "match.fd.table_name" msgstr "Table Name" msgid "match.fd.table_name.desc" -msgstr "Unique, alphanumeric name for matchgrid (will be prefixed mg_ for actual table name)" +msgstr "Unique name for matchgrid, must be a valid SQL identifier (will be prefixed mg_ for actual table name)" msgid "match.fd.username" msgstr "Username" diff --git a/app/src/Model/Table/AttributesTable.php b/app/src/Model/Table/AttributesTable.php index 4774c2387..ad9035024 100644 --- a/app/src/Model/Table/AttributesTable.php +++ b/app/src/Model/Table/AttributesTable.php @@ -36,6 +36,7 @@ class AttributesTable extends Table { use \App\Lib\Traits\AutoViewVarsTrait; use \App\Lib\Traits\MatchgridLinkTrait; use \App\Lib\Traits\PrimaryLinkTrait; + use \App\Lib\Traits\ValidationTrait; /** * Perform Cake Model initialization. @@ -93,8 +94,8 @@ public function validationDefault(Validator $validator) { $validator->add( 'name', 'content', - [ 'rule' => [ 'alphaNumeric' ], - 'message' => __('match.er.val.alphanum') ] + [ 'rule' => [ 'validateSqlIdentifier' ], + 'provider' => 'table' ] ); $validator->notEmpty('name'); diff --git a/app/src/Model/Table/MatchgridsTable.php b/app/src/Model/Table/MatchgridsTable.php index d9b996f37..7835bfd2d 100644 --- a/app/src/Model/Table/MatchgridsTable.php +++ b/app/src/Model/Table/MatchgridsTable.php @@ -42,6 +42,7 @@ class MatchgridsTable extends Table { use \App\Lib\Traits\AutoViewVarsTrait; use \App\Lib\Traits\MatchgridLinkTrait; use \App\Lib\Traits\PrimaryLinkTrait; + use \App\Lib\Traits\ValidationTrait; /** * Perform Cake Model initialization. @@ -178,7 +179,8 @@ public function validationDefault(Validator $validator) { $validator->add( 'table_name', 'content', - [ 'rule' => [ 'custom', '/^[a-zA-Z0-9_$]+/' ] ] + [ 'rule' => [ 'validateSqlIdentifier' ], + 'provider' => 'table' ] ); $validator->notEmpty('table_name');