From 33aef476b8631c11fa6a63b8197224cca044cd88 Mon Sep 17 00:00:00 2001 From: Roy Ronalds Date: Sun, 29 Jan 2023 00:06:01 -0500 Subject: [PATCH] Test filtering fixes. --- deploy/lib/Filter.php | 37 +++++++++++++++---------- deploy/lib/control/SignupController.php | 10 +++---- deploy/tests/unit/environment_test.php | 37 ++++++++++++++++--------- 3 files changed, 51 insertions(+), 33 deletions(-) diff --git a/deploy/lib/Filter.php b/deploy/lib/Filter.php index f703fc487..eb6e45b4b 100644 --- a/deploy/lib/Filter.php +++ b/deploy/lib/Filter.php @@ -5,7 +5,8 @@ /** * Filter & Sanitation wrappers */ -class Filter { +class Filter +{ /** * Return a casting with a result of a positive int, or else zero. * @@ -13,31 +14,37 @@ class Filter { * this function will cast strings with leading integers to those integers. * E.g. 555'sql-injection becomes 555 */ - public static function toNonNegativeInt($num) { + public static function toNonNegativeInt($num) + { return ((int)$num == $num && (int)$num > 0 ? (int)$num : 0); } /** * Casts to an integer anything that can be cast that way non-destructively, otherwise null. */ - public static function toInt($dirty) { + public static function toInt($dirty) + { return $dirty == (int) $dirty ? (int) $dirty : null; // Cast anything that can be non-destructively cast. } - public static function filter_string_polyfill(string $string): string { - $str = preg_replace('/\x00|<[^>]*>?/', '', $string); - return str_replace(["'", '"'], [''', '"'], $str); + public static function toAlphaNumeric($dirty) + { + return preg_replace('/[^a-zA-Z0-9]/', '', $dirty); } - /** - * Strip low and high ascii characters, leave standard keyboard characters - */ - public static function toSimple($dirty) { - return static::filter_string_polyfill(filter_var( - str_replace(['"', '\''], '', $dirty), - FILTER_SANITIZE_FULL_SPECIAL_CHARS, - FILTER_FLAG_STRIP_LOW | FILTER_FLAG_STRIP_HIGH - )); + public static function toAllowableUsername($dirty) + { + return preg_replace('/[^a-zA-Z0-9_-]/', '', $dirty); + } + + public static function toEmail($dirty) + { + return filter_var($dirty, FILTER_SANITIZE_EMAIL); + } + + public static function stripHighUtf8($dirty) + { + return filter_var($dirty, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_HIGH | FILTER_FLAG_STRIP_LOW); } } diff --git a/deploy/lib/control/SignupController.php b/deploy/lib/control/SignupController.php index 260564bb7..d2bd67477 100644 --- a/deploy/lib/control/SignupController.php +++ b/deploy/lib/control/SignupController.php @@ -160,7 +160,7 @@ private function validateSignupRequest($p_request) { throw new \RuntimeException('Phase 1 Incomplete: You did not correctly fill out all the necessary information.', 0); } - if ($p_request->enteredPass !== Filter::toSimple($p_request->enteredPass)) { + if ($p_request->enteredPass !== Filter::stripHighUtf8($p_request->enteredPass)) { throw new \RuntimeException("Sorry, there seem to be some very special non-standard characters in your password that we don't allow."); } @@ -191,12 +191,12 @@ private function validateSignupRequest($p_request) { */ private function buildSignupRequest($p_request) { $signupRequest = new \stdClass(); - $signupRequest->enteredName = Filter::toSimple(trim($p_request->get('send_name') ?? '')); - $signupRequest->enteredEmail = Filter::toSimple(trim($p_request->get('send_email') ?? '')); + $signupRequest->enteredName = Filter::toAllowableUsername(trim($p_request->get('send_name') ?? '')); + $signupRequest->enteredEmail = Filter::toEmail(trim($p_request->get('send_email') ?? '')); $signupRequest->enteredClass = strtolower(trim($p_request->get('send_class') ?? '')); $signupRequest->enteredReferral = trim($p_request->get('referred_by', $p_request->get('referrer')) ?? ''); - $signupRequest->enteredPass = Filter::toSimple($p_request->get('key') ?? ''); - $signupRequest->enteredCPass = Filter::toSimple($p_request->get('cpass') ?? ''); + $signupRequest->enteredPass = Filter::stripHighUtf8($p_request->get('key') ?? ''); + $signupRequest->enteredCPass = Filter::stripHighUtf8($p_request->get('cpass') ?? ''); $signupRequest->clientIP = $p_request->getClientIp(); // Fallback to key for class diff --git a/deploy/tests/unit/environment_test.php b/deploy/tests/unit/environment_test.php index d186000fb..e050b36c8 100644 --- a/deploy/tests/unit/environment_test.php +++ b/deploy/tests/unit/environment_test.php @@ -6,13 +6,18 @@ use NinjaWars\core\environment\RequestWrapper; use NinjaWars\core\Filter; -class TestInput extends NWTest { - public function setUp(): void { +/** + * @group environment + */ +class TestInput extends \NWTest +{ + public function setUp(): void + { parent::setUp(); $get = [ 'id' => 7, 'ninja_name' => 5, - 'some_negative_int'=> -444, + 'some_negative_int' => -444, 'some_int' => 66, 'garbage_field' => 'Robert\'); drop table students; --' ]; @@ -20,7 +25,7 @@ public function setUp(): void { $post = [ 'hidden_post' => 1, 'post_post_field' => 'Bob', - 'post_negative_int'=> -234, + 'post_negative_int' => -234, 'post_some_int' => 34, 'post_garbage_field' => 'Robert\'); drop table students; --' ]; @@ -30,26 +35,30 @@ public function setUp(): void { RequestWrapper::inject($request); // Pass a request to be used by tests } - public function tearDown(): void { + public function tearDown(): void + { RequestWrapper::destroy(); parent::tearDown(); } - public function testInputWithinEnvironment() { + public function testInputWithinEnvironment() + { $id = RequestWrapper::getPostOrGet('id'); $this->assertEquals(7, $id); $default_result = RequestWrapper::getPostOrGet('doesnotexist', 5); $this->assertEquals(5, $default_result); } - public function testPostWithinMockedEnvironment() { + public function testPostWithinMockedEnvironment() + { $posted = RequestWrapper::getPost('post_post_field', 'Bob'); $this->assertEquals('Bob', $posted); $default = RequestWrapper::getPost('blah_doesnt_exist', 7777); $this->assertEquals(7777, $default); } - public function testNonNegativeInt() { + public function testNonNegativeInt() + { $this->assertEquals(4, Filter::toNonNegativeInt(4)); $this->assertEquals(0, Filter::toNonNegativeInt(-4)); $this->assertEquals(0, Filter::toNonNegativeInt(4.1)); @@ -62,7 +71,8 @@ public function testNonNegativeInt() { /** * */ - public function testSanitizeToInt() { + public function testSanitizeToInt() + { $this->assertEquals(4, Filter::toInt(4)); $this->assertEquals(-4, Filter::toInt(-4)); $this->assertNull(Filter::toInt(4.1)); @@ -72,7 +82,8 @@ public function testSanitizeToInt() { $this->assertEquals(0, Filter::toInt(0)); } - public function testToInt() { + public function testToInt() + { $this->assertEquals(4, Filter::toInt(4)); $this->assertEquals(-4, Filter::toInt(-4)); $this->assertNull(Filter::toInt(4.1)); @@ -82,8 +93,8 @@ public function testToInt() { $this->assertEquals(0, Filter::toInt(0)); } - public function testFilterToSimple() { - $this->assertEquals('boba', Filter::toSimple("bob\0aä\x80")); - $this->assertEquals("!@#^&()_+--", Filter::toSimple("!@#^&()_+'''\"\"''--")); + public function testFilterStripHighUtf8() + { + $this->assertEquals('boba', Filter::stripHighUtf8("bob\0aä\x80")); } }