From 3b4950d535ac0ac64bed8e19587c600be4c44d7a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 31 Aug 2021 18:16:00 +0200 Subject: [PATCH] Ssl: add input validation to all methods None of the methods in the `WpOrg\Requests\Ssl` class did any input validation. This could lead to various PHP errors and becomes all the more relevant due to the new "passing null to non-nullable" deprecation notice on PHP 8.1. This commit adds simple input validation to all methods within the `WpOrg\Requests\Ssl` class and will throw an `WpOrg\Requests\Exception\InvalidArgument` exception if a parameter does not comply with the expectations. Note: for the `$cert` array parameter in the `Ssl::verify_certificate()` method, I considered adding an `array` type declaration, however, for consistency, always throwing the Requests native `InvalidArgument` exception seemed the better choice. Includes adding tests for the new exception. --- src/Ssl.php | 23 +++++++++++++ tests/SslTest.php | 86 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/Ssl.php b/src/Ssl.php index 3161f7a2d..7e138e44f 100644 --- a/src/Ssl.php +++ b/src/Ssl.php @@ -8,6 +8,9 @@ namespace WpOrg\Requests; +use WpOrg\Requests\Exception\InvalidArgument; +use WpOrg\Requests\Utility\InputValidator; + /** * SSL utilities for Requests * @@ -28,8 +31,18 @@ final class Ssl { * @param string $host Host name to verify against * @param array $cert Certificate data from openssl_x509_parse() * @return bool + * @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $host argument is not a string or a stringable object. + * @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $cert argument is not an array or array accessible. */ public static function verify_certificate($host, $cert) { + if (InputValidator::is_string_or_stringable($host) === false) { + throw InvalidArgument::create(1, '$host', 'string|Stringable', gettype($host)); + } + + if (InputValidator::has_array_access($cert) === false) { + throw InvalidArgument::create(2, '$cert', 'array|ArrayAccess', gettype($cert)); + } + $has_dns_alt = false; // Check the subjectAltName @@ -82,8 +95,13 @@ public static function verify_certificate($host, $cert) { * * @param string $reference Reference dNSName * @return boolean Is the name valid? + * @throws \WpOrg\Requests\Exception\InvalidArgument When the passed argument is not a string or a stringable object. */ public static function verify_reference_name($reference) { + if (InputValidator::is_string_or_stringable($reference) === false) { + throw InvalidArgument::create(1, '$reference', 'string|Stringable', gettype($reference)); + } + $parts = explode('.', $reference); // Check the first part of the name @@ -118,8 +136,13 @@ public static function verify_reference_name($reference) { * @param string $host Requested host * @param string $reference dNSName to match against * @return boolean Does the domain match? + * @throws \WpOrg\Requests\Exception\InvalidArgument When either of the passed arguments is not a string or a stringable object. */ public static function match_domain($host, $reference) { + if (InputValidator::is_string_or_stringable($host) === false) { + throw InvalidArgument::create(1, '$host', 'string|Stringable', gettype($host)); + } + // Check if the reference is blocklisted first if (self::verify_reference_name($reference) !== true) { return false; diff --git a/tests/SslTest.php b/tests/SslTest.php index ff7d83b8a..327135240 100644 --- a/tests/SslTest.php +++ b/tests/SslTest.php @@ -2,6 +2,8 @@ namespace WpOrg\Requests\Tests; +use stdClass; +use WpOrg\Requests\Exception\InvalidArgument; use WpOrg\Requests\Ssl; use WpOrg\Requests\Tests\TestCase; @@ -392,4 +394,88 @@ public function dataVerifyCertificateWithInvalidCertificates() { ), ); } + + /** + * Tests receiving an exception when an invalid input type is passed as $host. + * + * @dataProvider dataInvalidInputType + * + * @covers ::verify_certificate + * + * @param mixed $input Input data. + * + * @return void + */ + public function testVerifyCertificateInvalidInputHost($input) { + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #1 ($host) must be of type string|Stringable'); + + Ssl::verify_certificate($input, array()); + } + + /** + * Tests receiving an exception when an invalid input type is passed as $cert. + * + * @dataProvider dataInvalidInputType + * + * @covers ::verify_certificate + * + * @param mixed $input Input data. + * + * @return void + */ + public function testVerifyCertificateInvalidInputCert($input) { + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #2 ($cert) must be of type array|ArrayAccess'); + + Ssl::verify_certificate('host', $input); + } + + /** + * Tests receiving an exception when an invalid input type is passed. + * + * @dataProvider dataInvalidInputType + * + * @covers ::verify_reference_name + * + * @param mixed $input Input data. + * + * @return void + */ + public function testVerifyReferenceNameInvalidInputType($input) { + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #1 ($reference) must be of type string|Stringable'); + + Ssl::verify_reference_name($input); + } + + /** + * Tests receiving an exception when an invalid input type is passed. + * + * @dataProvider dataInvalidInputType + * + * @covers ::match_domain + * + * @param mixed $input Input data. + * + * @return void + */ + public function testInvalidInputType($input) { + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #1 ($host) must be of type string|Stringable'); + + Ssl::match_domain($input, 'reference'); + } + + /** + * Data Provider. + * + * @return array + */ + public function dataInvalidInputType() { + return array( + 'null' => array(null), + 'plain object' => array(new stdClass()), + ); + } }