Skip to content

Commit

Permalink
Dw/huntr fix path traversal (#2931)
Browse files Browse the repository at this point in the history 
* Fix Huntr vuln with possible directory traversal
* Use `active_url` in Laravel validator
  • Loading branch information
@luceos
luceos committed Jun 21, 2021
1 parent d1e3855 commit a0152ff
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 4 deletions.
32 changes: 28 additions & 4 deletions src/User/Command/RegisterUserHandler.php
View file
Expand Up @@ -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
{
Expand All @@ -36,25 +38,30 @@ class RegisterUserHandler
/**
* @var UserValidator
*/
protected $validator;
protected $userValidator;

/**
* @var AvatarUploader
*/
protected $avatarUploader;
/**
* @var Factory
*/
private $validator;

/**
* @param Dispatcher $events
* @param SettingsRepositoryInterface $settings
* @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;
}

/**
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
215 changes: 215 additions & 0 deletions tests/integration/api/users/CreateTest.php
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}

0 comments on commit a0152ff

Please sign in to comment.