Skip to content

Commit

Permalink
Improve validation of field names associated with SQL identifiers (CO…
Browse files Browse the repository at this point in the history
…-1767)
  • Loading branch information
Benn Oshrin committed Sep 10, 2019
1 parent 7dcb3d4 commit 09d26bf
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 8 deletions.
98 changes: 98 additions & 0 deletions app/src/Lib/Traits/ValidationTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php
/**
* COmanage Validation Trait, shared between Match and Registry
*
* Portions licensed to the University Corporation for Advanced Internet
* Development, Inc. ("UCAID") under one or more contributor license agreements.
* See the NOTICE file distributed with this work for additional information
* regarding copyright ownership.
*
* UCAID licenses this file to you under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @link http://www.internet2.edu/comanage COmanage Project
* @package common
* @since COmanage Common v1.0.0
* @license Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
*/

/**
* THIS FILE IS MASTERED IN THE COMMON REPOSITORY.
*/

declare(strict_types = 1);

namespace App\Lib\Traits;

trait ValidationTrait {
/**
* Determine if a string submitted from a form is valid input.
*
* @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 validateInput($value, array $context) {
// By default, we'll accept anything except < and >. 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;
}
}
13 changes: 8 additions & 5 deletions app/src/Locale/en_US/default.po
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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}"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions app/src/Model/Table/AttributesTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');

Expand Down
4 changes: 3 additions & 1 deletion app/src/Model/Table/MatchgridsTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');

Expand Down

0 comments on commit 09d26bf

Please sign in to comment.