From a0152ffb1898860243637a701a917ef8ab639571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Klabbers?= Date: Mon, 21 Jun 2021 10:14:15 +0200 Subject: [PATCH] Dw/huntr fix path traversal (#2931) * Fix Huntr vuln with possible directory traversal * Use `active_url` in Laravel validator --- src/User/Command/RegisterUserHandler.php | 32 ++- tests/integration/api/users/CreateTest.php | 215 +++++++++++++++++++++ 2 files changed, 243 insertions(+), 4 deletions(-) diff --git a/src/User/Command/RegisterUserHandler.php b/src/User/Command/RegisterUserHandler.php index 5d0706a6dd..4c3bef5091 100644 --- a/src/User/Command/RegisterUserHandler.php +++ b/src/User/Command/RegisterUserHandler.php @@ -21,8 +21,10 @@ use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Support\Arr; use Illuminate\Support\Str; +use Illuminate\Validation\Factory; use Illuminate\Validation\ValidationException; use Intervention\Image\ImageManager; +use InvalidArgumentException; class RegisterUserHandler { @@ -36,12 +38,16 @@ class RegisterUserHandler /** * @var UserValidator */ - protected $validator; + protected $userValidator; /** * @var AvatarUploader */ protected $avatarUploader; + /** + * @var Factory + */ + private $validator; /** * @param Dispatcher $events @@ -49,12 +55,13 @@ class RegisterUserHandler * @param UserValidator $validator * @param AvatarUploader $avatarUploader */ - public function __construct(Dispatcher $events, SettingsRepositoryInterface $settings, UserValidator $validator, AvatarUploader $avatarUploader) + public function __construct(Dispatcher $events, SettingsRepositoryInterface $settings, UserValidator $userValidator, AvatarUploader $avatarUploader, Factory $validator) { $this->events = $events; $this->settings = $settings; - $this->validator = $validator; + $this->userValidator = $userValidator; $this->avatarUploader = $avatarUploader; + $this->validator = $validator; } /** @@ -101,7 +108,7 @@ public function handle(RegisterUser $command) new Saving($user, $actor, $data) ); - $this->validator->assertValid(array_merge($user->getAttributes(), compact('password'))); + $this->userValidator->assertValid(array_merge($user->getAttributes(), compact('password'))); $user->save(); @@ -134,8 +141,25 @@ private function applyToken(User $user, RegistrationToken $token) ); } + /** + * @throws InvalidArgumentException + */ private function uploadAvatarFromUrl(User $user, string $url) { + $urlValidator = $this->validator->make(compact('url'), [ + 'url' => 'required|active_url', + ]); + + if ($urlValidator->fails()) { + throw new InvalidArgumentException('Provided avatar URL must be a valid URI.', 503); + } + + $scheme = parse_url($url, PHP_URL_SCHEME); + + if (! in_array($scheme, ['http', 'https'])) { + throw new InvalidArgumentException("Provided avatar URL must have scheme http or https. Scheme provided was $scheme.", 503); + } + $image = (new ImageManager)->make($url); $this->avatarUploader->upload($user, $image); diff --git a/tests/integration/api/users/CreateTest.php b/tests/integration/api/users/CreateTest.php index ba9b7239ba..85c358ab8f 100644 --- a/tests/integration/api/users/CreateTest.php +++ b/tests/integration/api/users/CreateTest.php @@ -12,6 +12,7 @@ use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Testing\integration\RetrievesAuthorizedUsers; use Flarum\Testing\integration\TestCase; +use Flarum\User\RegistrationToken; use Flarum\User\User; class CreateTest extends TestCase @@ -168,4 +169,218 @@ public function disabling_sign_up_prevents_user_creation() $settings->set('allow_sign_up', true); } + + /** + * @test + */ + public function cannot_create_user_with_invalid_avatar_uri_scheme() + { + // Boot app + $this->app(); + + $regTokens = []; + + // Add registration tokens that should cause a failure + $regTokens[] = [ + 'token' => RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'file://localhost/etc/passwd' + ], []), + 'scheme' => 'file' + ]; + + $regTokens[] = [ + 'token' => RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'ftp://localhost/image.png' + ], []), + 'scheme' => 'ftp' + ]; + + // Test each reg token + foreach ($regTokens as $regToken) { + $regToken['token']->saveOrFail(); + + // Call the registration endpoint + $response = $this->send( + $this->request( + 'POST', + '/api/users', + [ + 'json' => [ + 'data' => [ + 'attributes' => [ + 'token' => $regToken['token']->token, + ], + ] + ], + ] + )->withAttribute('bypassCsrfToken', true) + ); + + // The response body should contain details about the invalid URI + $body = (string) $response->getBody(); + $this->assertJson($body); + $decodedBody = json_decode($body, true); + + $this->assertEquals(500, $response->getStatusCode()); + + $firstError = $decodedBody['errors'][0]; + + // Check that the error is an invalid URI + $this->assertStringStartsWith('InvalidArgumentException: Provided avatar URL must have scheme http or https. Scheme provided was '.$regToken['scheme'].'.', $firstError['detail']); + } + } + + /** + * @test + */ + public function cannot_create_user_with_invalid_avatar_uri() + { + // Boot app + $this->app(); + + $regTokens = []; + + // Add registration tokens that should cause a failure + $regTokens[] = RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'https://127.0.0.1/image.png' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'https://192.168.0.1/image.png' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => '../image.png' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '1', [ + 'username' => 'test', + 'email' => 'test@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'image.png' + ], []); + + // Test each reg token + foreach ($regTokens as $regToken) { + $regToken->saveOrFail(); + + // Call the registration endpoint + $response = $this->send( + $this->request( + 'POST', + '/api/users', + [ + 'json' => [ + 'data' => [ + 'attributes' => [ + 'token' => $regToken->token, + ], + ] + ], + ] + )->withAttribute('bypassCsrfToken', true) + ); + + // The response body should contain details about the invalid URI + $body = (string) $response->getBody(); + $this->assertJson($body); + $decodedBody = json_decode($body, true); + + $this->assertEquals(500, $response->getStatusCode()); + + $firstError = $decodedBody['errors'][0]; + + // Check that the error is an invalid URI + $this->assertStringStartsWith('InvalidArgumentException: Provided avatar URL must be a valid URI.', $firstError['detail']); + } + } + + /** + * @test + */ + public function can_create_user_with_valid_avatar_uri() + { + // Boot app + $this->app(); + + $regTokens = []; + + // Add registration tokens that should work fine + $regTokens[] = RegistrationToken::generate('flarum', '1', [ + 'username' => 'test1', + 'email' => 'test1@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'https://via.placeholder.com/150.png' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '2', [ + 'username' => 'test2', + 'email' => 'test2@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'https://via.placeholder.com/150.jpg' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '3', [ + 'username' => 'test3', + 'email' => 'test3@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'https://via.placeholder.com/150.gif' + ], []); + + $regTokens[] = RegistrationToken::generate('flarum', '4', [ + 'username' => 'test4', + 'email' => 'test4@machine.local', + 'is_email_confirmed' => 1, + 'avatar_url' => 'http://via.placeholder.com/150.png' + ], []); + + /** + * Test each reg token. + * + * @var RegistrationToken $regToken + */ + foreach ($regTokens as $regToken) { + $regToken->saveOrFail(); + + // Call the registration endpoint + $response = $this->send( + $this->request( + 'POST', + '/api/users', + [ + 'json' => [ + 'data' => [ + 'attributes' => [ + 'token' => $regToken->token, + ], + ] + ], + ] + )->withAttribute('bypassCsrfToken', true) + ); + + $this->assertEquals(201, $response->getStatusCode()); + + $user = User::where('username', $regToken->user_attributes['username'])->firstOrFail(); + + $this->assertEquals($regToken->user_attributes['is_email_confirmed'], $user->is_email_confirmed); + $this->assertEquals($regToken->user_attributes['username'], $user->username); + $this->assertEquals($regToken->user_attributes['email'], $user->email); + } + } }