Skip to content

Commit

Permalink
feat: Validate extraSettings especially the regex for custom short …
Browse files Browse the repository at this point in the history
…input

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Mar 5, 2023
1 parent d87a20d commit 0edb495
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 0 deletions.
27 changes: 27 additions & 0 deletions lib/Constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,33 @@ class Constants {
self::ANSWER_TYPE_TIME => 'H:i'
];

/**
* !! Keep in sync with src/models/Constants.js !!
*/

// Allowed short input types
public const SHORT_INPUT_TYPES = [
'text',
'tel',
'email',
'custom',
'number'
];

// This are allowed extra settings
public const EXTRA_SETTINGS_DROPDOWN = [
'shuffleOptions' => true
];

public const EXTRA_SETTINGS_MULTIPLE = [
'shuffleOptions' => true
];

public const EXTRA_SETTINGS_SHORT = [
'type' => '',
'regex' => ''
];

/**
* !! Keep in sync with src/mixins/ShareTypes.js !!
*/
Expand Down
4 changes: 4 additions & 0 deletions lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,10 @@ public function updateQuestion(int $id, array $keyValuePairs): DataResponse {
throw new OCSForbiddenException('Please use reorderQuestions() to change order');
}

if (key_exists('extraSettings', $keyValuePairs) && ! $this->formsService->validateExtraSettings($keyValuePairs['extraSettings'], $question->getType())) {
throw new OCSBadRequestException('Invalid extraSettings, will not update.');
}

// Create QuestionEntity with given Params & Id.
$question = Question::fromParams($keyValuePairs);
$question->setId($id);
Expand Down
76 changes: 76 additions & 0 deletions lib/Service/FormsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -539,4 +539,80 @@ public function setLastUpdatedTimestamp(int $formId): void {
$form->setLastUpdated(time());
$this->formMapper->update($form);
}

/*
* Validates the extraSettings
*
* @param array $extraSettings input extra settings
* @param string $questionType the question type
* @return bool if the settings are valid
*/
public function validateExtraSettings(array $extraSettings, string $questionType) {
if (count($extraSettings) === 0) {
return true;
}

// Ensure only allowed keys are set
switch ($questionType) {
case Constants::ANSWER_TYPE_DROPDOWN:
$allowed = Constants::EXTRA_SETTINGS_DROPDOWN;
break;
case Constants::ANSWER_TYPE_MULTIPLE:
case Constants::ANSWER_TYPE_MULTIPLEUNIQUE:
$allowed = Constants::EXTRA_SETTINGS_MULTIPLE;
break;
case Constants::ANSWER_TYPE_SHORT:
$allowed = Constants::EXTRA_SETTINGS_SHORT;
break;
default:
$allowed = [];
}
$diff = array_diff_key($extraSettings, $allowed);
if (count($diff) > 0) {
return false;
}

// Special handling of regex
if ($questionType === Constants::ANSWER_TYPE_SHORT && isset($extraSettings['type'])) {
// Validate short input types
if (!in_array($extraSettings['type'], Constants::SHORT_INPUT_TYPES)) {
return false;
}

// For custom validation we need to sanitize the regex
if ($extraSettings['type'] === 'custom' && isset($extraSettings['regex'])) {
$VALID_REGEX = '/^\/(.+)\/([smi]{0,3})$/';
$REGEX_UNESCAPED_SLASHES = '/(?<=[^\\\\])(\\\\)*\//';

// must be a string
if (!is_string($extraSettings['regex'])) {
return false;
}

// empty regex matches every thing, this happens also when a new question is created
if (strlen($extraSettings['regex']) === 0) {
return true;
}

// Validate pattern
$matches = [];
if (@preg_match($VALID_REGEX, $extraSettings['regex'], $matches) !== 1) {
// only pattern with delimiters and supported modifiers (by PHP *and* JS)
return false;
}

if (@preg_match($REGEX_UNESCAPED_SLASHES, $matches[1]) === 1) {
// unescaped slashes
return false;
}


if (@preg_match($extraSettings['regex'], 'some string') === false) {
// regex is not valid
return false;
}
}
}
return true;
}
}
23 changes: 23 additions & 0 deletions src/models/Constants.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,28 @@
/**
* @copyright Copyright (c) 2023 Ferdinand Thiessen <[email protected]>
*
* @author Ferdinand Thiessen <[email protected]>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

import { translate as t } from '@nextcloud/l10n'

// !! Keep in SYNC with lib/Constants.php !!
export const SHORT_INPUT_TYPES = [
'text',
'tel',
Expand Down
62 changes: 62 additions & 0 deletions tests/Unit/Service/FormsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1269,4 +1269,66 @@ public function testNotifyNewShares(int $shareType, string $shareWith, string $e

$this->formsService->notifyNewShares($form, $share);
}

public function dataValidateExtraSettings() {
return [
'invalid key' => [
'extraSettings' => [
'some' => 'value'
],
'questionType' => Constants::ANSWER_TYPE_LONG,
'expected' => false
],
'valid key' => [
'extraSettings' => [
'shuffleOptions' => true
],
'questionType' => Constants::ANSWER_TYPE_MULTIPLE,
'expected' => true
],
'invalid subtype' => [
'extraSettings' => [
'type' => 'iban',
],
'questionType' => Constants::ANSWER_TYPE_SHORT,
'expected' => false
],
'valid regex' => [
'extraSettings' => [
'type' => 'custom',
'regex' => '/^[a-z]{3,}/m'
],
'questionType' => Constants::ANSWER_TYPE_SHORT,
'expected' => true
],
'invalid regex modifier' => [
'extraSettings' => [
'type' => 'custom',
'regex' => '/^[a-z]{3,}/gm'
],
'questionType' => Constants::ANSWER_TYPE_SHORT,
'expected' => false
],
'invalid regex' => [
'extraSettings' => [
'type' => 'custom',
'regex' => '['
],
'questionType' => Constants::ANSWER_TYPE_SHORT,
'rval' => false
]
];
}

/**
* @dataProvider dataValidateExtraSettings
*
* @param array $extraSettings input settings
* @param string $questionType question type
* @param bool $expected expected return value
*
*/
public function testValidateExtraSettings(array $extraSettings, string $questionType, bool $expected) {
$this->assertEquals($expected, $this->formsService->validateExtraSettings($extraSettings, $questionType));
}
}

0 comments on commit 0edb495

Please sign in to comment.