From 0edb495c5f20fb3ee4db63622a64b51b9d239030 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sun, 5 Mar 2023 22:16:00 +0100 Subject: [PATCH] feat: Validate `extraSettings` especially the regex for custom short input Signed-off-by: Ferdinand Thiessen --- lib/Constants.php | 27 +++++++++ lib/Controller/ApiController.php | 4 ++ lib/Service/FormsService.php | 76 +++++++++++++++++++++++++ src/models/Constants.js | 23 ++++++++ tests/Unit/Service/FormsServiceTest.php | 62 ++++++++++++++++++++ 5 files changed, 192 insertions(+) diff --git a/lib/Constants.php b/lib/Constants.php index 60af263ff..344f664f9 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -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 !! */ diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index befcbea9d..610d1bf94 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -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); diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index ee587beab..9cd28f852 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -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; + } } diff --git a/src/models/Constants.js b/src/models/Constants.js index 01c4e06ef..6909e4bc2 100644 --- a/src/models/Constants.js +++ b/src/models/Constants.js @@ -1,5 +1,28 @@ +/** + * @copyright Copyright (c) 2023 Ferdinand Thiessen + * + * @author Ferdinand Thiessen + * + * @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 . + * + */ + import { translate as t } from '@nextcloud/l10n' +// !! Keep in SYNC with lib/Constants.php !! export const SHORT_INPUT_TYPES = [ 'text', 'tel', diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 0494714df..0d9646c9e 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -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)); + } }