diff --git a/CHANGELOG.md b/CHANGELOG.md index d36835b..777fa41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). -## [Unreleased] - XXXX-XX-XX +## [0.5.4] - 2023-08-31 + +### Fixed + +- [#359](https://github.com/owncloud/oauth2/pull/359) - fix: harden subdomain validation ## [0.5.3] - 2022-06-02 @@ -140,7 +144,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Verify Bearer token even if the session is still valid - [#53](https://github.com/owncloud/oauth2/pull/53) - Use displayname on switch user screen - [#90](https://github.com/owncloud/oauth2/pull/90) -[Unreleased]: https://github.com/owncloud/oauth2/compare/v0.5.3...master +[Unreleased]: https://github.com/owncloud/oauth2/compare/v0.5.4...master +[0.5.4]: https://github.com/owncloud/oauth2/compare/v0.5.3...v0.5.4 [0.5.3]: https://github.com/owncloud/oauth2/compare/v0.5.2...v0.5.3 [0.5.2]: https://github.com/owncloud/oauth2/compare/v0.5.1...v0.5.2 [0.5.1]: https://github.com/owncloud/oauth2/compare/v0.5.0...v0.5.1 diff --git a/appinfo/info.xml b/appinfo/info.xml index e5523d9..f66a7ad 100755 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -24,7 +24,7 @@ When using OAuth2 a unique access token is generated for each device or third pa - [OAuth protocol web page](https://oauth.net/2/) AGPL Project Seminar "sciebo@Learnweb" of the University of Münster, Thomas Müller - 0.5.3 + 0.5.4 OAuth2 security https://github.com/owncloud/oauth2 diff --git a/lib/Utilities.php b/lib/Utilities.php index f2f6f78..35a07a8 100644 --- a/lib/Utilities.php +++ b/lib/Utilities.php @@ -47,7 +47,7 @@ public static function generateRandom(): string { * * @return boolean True if the redirection URI is valid, false otherwise. */ - public static function validateRedirectUri($expected, $actual, $allowSubdomains) { + public static function validateRedirectUri($expected, $actual, $allowSubdomains): bool { $validatePort = true; if (\strpos($expected, 'http://localhost:*') === 0) { $expected = 'http://localhost' . \substr($expected, 18); @@ -60,13 +60,7 @@ public static function validateRedirectUri($expected, $actual, $allowSubdomains) return false; } - if ($allowSubdomains) { - if (\strcmp($expectedUrl->hostname, $actualUrl->hostname) !== 0 - && \strcmp($expectedUrl->hostname, \str_replace(\explode('.', $actualUrl->hostname)[0] . '.', '', $actualUrl->hostname)) !== 0 - ) { - return false; - } - } elseif (\strcmp($expectedUrl->hostname, $actualUrl->hostname) !== 0) { + if (!self::validateDomain($expectedUrl, $actualUrl, $allowSubdomains)) { return false; } @@ -96,12 +90,28 @@ public static function removeWildcardPort($redirectUri): string { } public static function isValidUrl($redirectUri): bool { - $redirectUri = Utilities::removeWildcardPort($redirectUri); + $redirectUri = self::removeWildcardPort($redirectUri); return (\filter_var($redirectUri, FILTER_VALIDATE_URL) !== false); } // See https://tools.ietf.org/pdf/rfc7636.pdf#56 - public static function base64url_encode($data) { + public static function base64url_encode($data): string { return \rtrim(\strtr(\base64_encode($data), '+/', '-_'), '='); } + + private static function validateDomain(URL $expectedUrl, URL $actualUrl, bool $allowSubdomains): bool { + if (!$allowSubdomains) { + return \strcmp($expectedUrl->hostname, $actualUrl->hostname) === 0; + } + + $expectedUrlParts = array_reverse(explode('.', $expectedUrl->hostname)); + $actualUrlParts = array_reverse(explode('.', $actualUrl->hostname)); + foreach ($expectedUrlParts as $i => $p) { + if ($p !== $actualUrlParts[$i]) { + return false; + } + } + + return true; + } } diff --git a/sonar-project.properties b/sonar-project.properties index 21f3eb1..76d47db 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,7 +1,7 @@ # Organization and project keys are displayed in the right sidebar of the project homepage sonar.organization=owncloud-1 sonar.projectKey=owncloud_oauth2 -sonar.projectVersion=0.5.3 +sonar.projectVersion=0.5.4 sonar.host.url=https://sonarcloud.io # ===================================================== diff --git a/tests/unit/UtilitiesTest.php b/tests/unit/UtilitiesTest.php index 3b67994..539bc6c 100755 --- a/tests/unit/UtilitiesTest.php +++ b/tests/unit/UtilitiesTest.php @@ -23,7 +23,7 @@ use PHPUnit\Framework\TestCase; class UtilitiesTest extends TestCase { - public function testGenerateRandom() { + public function testGenerateRandom(): void { $random = Utilities::generateRandom(); $this->assertEquals(64, \strlen($random)); @@ -41,7 +41,9 @@ public function providesUrlsToValidate(): array { [false, 'https://owncloud.org:80/tests?q=1', 'https://owncloud.org:80/test?q=1', false], [false, 'https://owncloud.org:80/test?q=1', 'https://owncloud.org:80/test?q=0', false], [true, 'http://localhost:*/test?q=1', 'http://localhost:12345/test?q=1', false], - [false, 'http://excepted.com', 'http://aaa\@excepted.com', false] + [false, 'http://excepted.com', 'http://aaa\@excepted.com', false], + [false, 'https://trustedclient.com', 'https://munity.trustedclient.community.', true], + [false, 'https://trustedclient.com', 'https://munity.trustedclient.community', true] ]; } @@ -52,7 +54,7 @@ public function providesUrlsToValidate(): array { * @param $actualRedirect * @param $allowSubDomain */ - public function testValidateRedirectUri($expectedResult, $expectedRedirect, $actualRedirect, $allowSubDomain) { + public function testValidateRedirectUri($expectedResult, $expectedRedirect, $actualRedirect, $allowSubDomain): void { $this->assertEquals( $expectedResult, Utilities::validateRedirectUri( @@ -79,11 +81,11 @@ public function providesUrls(): array { * @param $expected * @param $url */ - public function testIsValidUrl($expected, $url) { + public function testIsValidUrl($expected, $url): void { $this->assertEquals($expected, Utilities::isValidUrl($url)); } - public function testBase64url_encode() { + public function testBase64url_encode(): void { $data = \random_bytes(32); $encoded = Utilities::base64url_encode($data); $this->assertEquals(43, \strlen($encoded));