From 170f3656e1062e2d6a76f3bb7c7982aab1a4eecd Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Thu, 23 Jan 2025 15:00:54 +0000 Subject: [PATCH] fix: redirect validator v2 --- app/controllers/api/account.php | 16 ++--- app/controllers/api/teams.php | 4 +- app/controllers/mock.php | 6 +- src/Appwrite/GraphQL/Types/Mapper.php | 1 + src/Appwrite/Network/Validator/Redirect.php | 70 +++++++++++++++++++ .../Specification/Format/OpenAPI3.php | 1 + .../Specification/Format/Swagger2.php | 1 + .../unit/Network/Validators/RedirectTest.php | 43 ++++++++++++ 8 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 src/Appwrite/Network/Validator/Redirect.php create mode 100644 tests/unit/Network/Validators/RedirectTest.php diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index bd9562110f..4de82ef9b9 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -21,6 +21,7 @@ use Appwrite\Event\Usage; use Appwrite\Extend\Exception; use Appwrite\Hooks\Hooks; use Appwrite\Network\Validator\Email; +use Appwrite\Network\Validator\Host; use Appwrite\OpenSSL\OpenSSL; use Appwrite\SDK\AuthType; use Appwrite\SDK\ContentType; @@ -59,7 +60,6 @@ use Utopia\System\System; use Utopia\Validator\ArrayList; use Utopia\Validator\Assoc; use Utopia\Validator\Boolean; -use Utopia\Validator\Host; use Utopia\Validator\Text; use Utopia\Validator\URL; use Utopia\Validator\WhiteList; @@ -1188,8 +1188,8 @@ App::get('/v1/account/sessions/oauth2/:provider') ->label('abuse-limit', 50) ->label('abuse-key', 'ip:{ip}') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('oAuthProviders')), true), 'OAuth2 Provider. Currently, supported providers are: ' . \implode(', ', \array_keys(\array_filter(Config::getParam('oAuthProviders'), fn ($node) => (!$node['mock'])))) . '.') - ->param('success', '', fn ($clients) => new Host($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) - ->param('failure', '', fn ($clients) => new Host($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('success', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('failure', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) ->param('scopes', [], new ArrayList(new Text(APP_LIMIT_ARRAY_ELEMENT_SIZE), APP_LIMIT_ARRAY_PARAMS_SIZE), 'A list of custom OAuth2 scopes. Check each provider internal docs for a list of supported scopes. Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' scopes are allowed, each ' . APP_LIMIT_ARRAY_ELEMENT_SIZE . ' characters long.', true) ->inject('request') ->inject('response') @@ -1784,8 +1784,8 @@ App::get('/v1/account/tokens/oauth2/:provider') ->label('abuse-limit', 50) ->label('abuse-key', 'ip:{ip}') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('oAuthProviders')), true), 'OAuth2 Provider. Currently, supported providers are: ' . \implode(', ', \array_keys(\array_filter(Config::getParam('oAuthProviders'), fn ($node) => (!$node['mock'])))) . '.') - ->param('success', '', fn ($clients) => new Host($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) - ->param('failure', '', fn ($clients) => new Host($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('success', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('failure', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) ->param('scopes', [], new ArrayList(new Text(APP_LIMIT_ARRAY_ELEMENT_SIZE), APP_LIMIT_ARRAY_PARAMS_SIZE), 'A list of custom OAuth2 scopes. Check each provider internal docs for a list of supported scopes. Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' scopes are allowed, each ' . APP_LIMIT_ARRAY_ELEMENT_SIZE . ' characters long.', true) ->inject('request') ->inject('response') @@ -1864,7 +1864,7 @@ App::post('/v1/account/tokens/magic-url') ->label('abuse-key', ['url:{url},email:{param-email}', 'url:{url},ip:{ip}']) ->param('userId', '', new CustomId(), 'Unique Id. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') ->param('email', '', new Email(), 'User email.') - ->param('url', '', fn ($clients) => new Host($clients), 'URL to redirect the user back to your app from the magic URL login. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the magic URL login. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) ->param('phrase', false, new Boolean(), 'Toggle for security phrase. If enabled, email will be send with a randomly generated phrase and the phrase will also be included in the response. Confirming phrases match increases the security of your authentication flow.', true) ->inject('request') ->inject('response') @@ -3157,7 +3157,7 @@ App::post('/v1/account/recovery') ->label('abuse-limit', 10) ->label('abuse-key', ['url:{url},email:{param-email}', 'url:{url},ip:{ip}']) ->param('email', '', new Email(), 'User email.') - ->param('url', '', fn ($clients) => new Host($clients), 'URL to redirect the user back to your app from the recovery email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) + ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the recovery email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) ->inject('request') ->inject('response') ->inject('user') @@ -3432,7 +3432,7 @@ App::post('/v1/account/verification') )) ->label('abuse-limit', 10) ->label('abuse-key', 'url:{url},userId:{userId}') - ->param('url', '', fn ($clients) => new Host($clients), 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) // TODO add built-in confirm page + ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) // TODO add built-in confirm page ->inject('request') ->inject('response') ->inject('project') diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index e525086ebf..63a29f279d 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -11,6 +11,7 @@ use Appwrite\Event\Messaging; use Appwrite\Event\Usage; use Appwrite\Extend\Exception; use Appwrite\Network\Validator\Email; +use Appwrite\Network\Validator\Redirect; use Appwrite\Platform\Workers\Deletes; use Appwrite\SDK\AuthType; use Appwrite\SDK\ContentType; @@ -49,7 +50,6 @@ use Utopia\Locale\Locale; use Utopia\System\System; use Utopia\Validator\ArrayList; use Utopia\Validator\Assoc; -use Utopia\Validator\Host; use Utopia\Validator\Text; use Utopia\Validator\WhiteList; @@ -455,7 +455,7 @@ App::post('/v1/teams/:teamId/memberships') } return new ArrayList(new Key(), APP_LIMIT_ARRAY_PARAMS_SIZE); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](https://appwrite.io/docs/permissions). Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' roles are allowed, each 32 characters long.', false, ['project']) - ->param('url', '', fn ($clients) => new Host($clients), 'URL to redirect the user back to your app from the invitation email. This parameter is not required when an API key is supplied. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) // TODO add our own built-in confirm page + ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the invitation email. This parameter is not required when an API key is supplied. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) // TODO add our own built-in confirm page ->param('name', '', new Text(128), 'Name of the new team member. Max length: 128 chars.', true) ->inject('response') ->inject('project') diff --git a/app/controllers/mock.php b/app/controllers/mock.php index 16d8e03841..eb108ac61a 100644 --- a/app/controllers/mock.php +++ b/app/controllers/mock.php @@ -3,6 +3,7 @@ global $utopia, $request, $response; use Appwrite\Extend\Exception; +use Appwrite\Network\Validator\Redirect; use Appwrite\Utopia\Request; use Appwrite\Utopia\Response; use Utopia\App; @@ -14,7 +15,6 @@ use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; use Utopia\Database\Validator\UID; use Utopia\System\System; -use Utopia\Validator\Host; use Utopia\Validator\Text; use Utopia\Validator\WhiteList; use Utopia\VCS\Adapter\Git\GitHub; @@ -26,7 +26,7 @@ App::get('/v1/mock/tests/general/oauth2') ->label('docs', false) ->label('mock', true) ->param('client_id', '', new Text(100), 'OAuth2 Client ID.') - ->param('redirect_uri', '', new Host(['localhost']), 'OAuth2 Redirect URI.') // Important to deny an open redirect attack + ->param('redirect_uri', '', new Redirect(['localhost']), 'OAuth2 Redirect URI.') // Important to deny an open redirect attack ->param('scope', '', new Text(100), 'OAuth2 scope list.') ->param('state', '', new Text(1024), 'OAuth2 state.') ->inject('response') @@ -44,7 +44,7 @@ App::get('/v1/mock/tests/general/oauth2/token') ->param('client_id', '', new Text(100), 'OAuth2 Client ID.') ->param('client_secret', '', new Text(100), 'OAuth2 scope list.') ->param('grant_type', 'authorization_code', new WhiteList(['refresh_token', 'authorization_code']), 'OAuth2 Grant Type.', true) - ->param('redirect_uri', '', new Host(['localhost']), 'OAuth2 Redirect URI.', true) + ->param('redirect_uri', '', new Redirect(['localhost']), 'OAuth2 Redirect URI.', true) ->param('code', '', new Text(100), 'OAuth2 state.', true) ->param('refresh_token', '', new Text(100), 'OAuth2 refresh token.', true) ->inject('response') diff --git a/src/Appwrite/GraphQL/Types/Mapper.php b/src/Appwrite/GraphQL/Types/Mapper.php index e5056d0abc..c0bcb9b716 100644 --- a/src/Appwrite/GraphQL/Types/Mapper.php +++ b/src/Appwrite/GraphQL/Types/Mapper.php @@ -266,6 +266,7 @@ class Mapper case 'Appwrite\Utopia\Database\Validator\CustomId': case 'Utopia\Validator\Domain': case 'Appwrite\Network\Validator\Email': + case 'Appwrite\Network\Validator\Redirect': case 'Appwrite\Event\Validator\Event': case 'Appwrite\Event\Validator\FunctionEvent': case 'Utopia\Validator\HexColor': diff --git a/src/Appwrite/Network/Validator/Redirect.php b/src/Appwrite/Network/Validator/Redirect.php new file mode 100644 index 0000000000..a1fc310013 --- /dev/null +++ b/src/Appwrite/Network/Validator/Redirect.php @@ -0,0 +1,70 @@ +whitelist); + } + + /** + * Is valid + * + * Validation will pass if scheme is not http or https or host is in whitelist + * + * @param mixed $value + * @return bool + */ + public function isValid($value): bool + { + // `parse_url()` returns false for URLs with only a scheme. + // We need to check for this case separately. + if (preg_match('/^([a-z][a-z0-9+\.-]*):\/\/?$/i', $value, $matches)) { + $scheme = strtolower($matches[1]); + return $scheme !== 'javascript'; + } + + $url = \parse_url($value); + if ($url === false || !isset($url["scheme"])) { + return false; + } + + $scheme = strtolower($url["scheme"]); + if ($scheme === "javascript") { + return false; + } + + if (!\in_array($scheme, ["http", "https"])) { + return true; + } + + return parent::isValid($value); + } +} diff --git a/src/Appwrite/Specification/Format/OpenAPI3.php b/src/Appwrite/Specification/Format/OpenAPI3.php index bd5405539d..2eb9187882 100644 --- a/src/Appwrite/Specification/Format/OpenAPI3.php +++ b/src/Appwrite/Specification/Format/OpenAPI3.php @@ -375,6 +375,7 @@ class OpenAPI3 extends Format $node['schema']['format'] = 'email'; $node['schema']['x-example'] = 'email@example.com'; break; + case 'Appwrite\Network\Validator\Redirect': case 'Utopia\Validator\Host': case 'Utopia\Validator\URL': $node['schema']['type'] = $validator->getType(); diff --git a/src/Appwrite/Specification/Format/Swagger2.php b/src/Appwrite/Specification/Format/Swagger2.php index 7277e3ab2b..927cda27d1 100644 --- a/src/Appwrite/Specification/Format/Swagger2.php +++ b/src/Appwrite/Specification/Format/Swagger2.php @@ -393,6 +393,7 @@ class Swagger2 extends Format $node['format'] = 'email'; $node['x-example'] = 'email@example.com'; break; + case 'Appwrite\Network\Validator\Redirect': case 'Utopia\Validator\Host': case 'Utopia\Validator\URL': $node['type'] = $validator->getType(); diff --git a/tests/unit/Network/Validators/RedirectTest.php b/tests/unit/Network/Validators/RedirectTest.php new file mode 100644 index 0000000000..5bf18dd260 --- /dev/null +++ b/tests/unit/Network/Validators/RedirectTest.php @@ -0,0 +1,43 @@ + [[], "exp://192.168.0.1", true], + "only scheme with triple slash" => [[], "myapp:///", true], + "only scheme" => [[], "myapp://", true], + "javascript scheme" => [[], "javascript://alert(1)", false], + "invalid url" => [[], "192.168.0.1", false], + "scheme case + invalid host" => [ + ["notexample.com"], + "HTTPS://example.com", + false, + ], + "scheme case + valid host" => [ + ["example.com"], + "HTTPS://example.com", + true, + ], + ]; + } + + /** + * @dataProvider redirectsProvider + */ + public function testIsValid( + array $allowList, + string $value, + bool $expected + ): void { + $validator = new Redirect($allowList); + + $this->assertEquals($expected, $validator->isValid($value)); + } +}