archtechx/tenancy
1
0
Fork 0
mirror of https://github.com/archtechx/tenancy.git synced 2025-12-12 14:14:04 +00:00
Code Activity

reimplement TenantAssetsController::validatePath() (fixes #1143)

Browse source
This commit is contained in:
Samuel Štancl 2023-09-02 03:19:37 +02:00
parent 4af70d302f
commit caf2267a08
2 changed files with 92 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

36
src/Controllers/TenantAssetsController.php
View file

@ -4,6 +4,7 @@ declare(strict_types=1);
namespace Stancl\Tenancy\Controllers;
use Exception;
use Illuminate\Routing\Controller;
use Throwable;
@ -28,18 +29,41 @@ class TenantAssetsController extends Controller
}
/**
* Prevent path traversal attacks. This is generally a non-issue on modern
* webservers but it's still worth handling on the application level as well.
*
* @throws \Symfony\Component\HttpKernel\Exception\HttpException
*/
protected function validatePath(string|null $path): void
{
abort_if($path === null, 404);
$this->abortIf($path === null, 'Empty path');
$allowedRoot = storage_path('app/public');
$allowedRoot = realpath(storage_path('app/public'));
// Prevent path traversal attacks. This is generally a non-issue on modern
// webservers but it's still worth handling on the application level as well.
if (! str(realpath("{$allowedRoot}/{$path}"))->startsWith($allowedRoot)) {
abort(403);
// `storage_path('app/public')` doesn't exist, so it cannot contain files
$this->abortIf($allowedRoot === false, "Storage root doesn't exist");
$attemptedPath = realpath("{$allowedRoot}/{$path}");
// User is attempting to access a nonexistent file
$this->abortIf($attemptedPath === false, 'Accessing a nonexistent file');
// User is attempting to access a file outside the $allowedRoot folder
$this->abortIf(! str($attemptedPath)->startsWith($allowedRoot), 'Accessing a file outside the storage root');
}
protected function abortIf($condition, $exceptionMessage): void
{
if ($condition) {
if (app()->runningUnitTests()) {
// Makes testing the cause of the failure in validatePath() easier
throw new Exception($exceptionMessage);
} else {
// We always use 404 to avoid leaking information about the cause of the error
// e.g. when someone is trying to access a nonexistent file outside of the allowed
// root folder, we don't want to let the user know whether such a file exists or not.
abort(404);
}
}
}
}

68
tests/TenantAssetTest.php
View file

@ -4,6 +4,7 @@ declare(strict_types=1);
namespace Stancl\Tenancy\Tests;
use Exception;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Route;
use Illuminate\Support\Facades\Storage;
@ -134,24 +135,79 @@ class TenantAssetTest extends TestCase
$tenant = Tenant::create();
tenancy()->initialize($tenant);
$response = $this->get(tenant_asset(null), [
$this->withoutExceptionHandling();
$this->expectExceptionMessage('Empty path'); // outside tests this is a 404
$this->get(tenant_asset(null), [
'X-Tenant' => $tenant->id,
]);
$response->assertNotFound();
}
public function test_asset_controller_returns_a_403_when_an_invalid_path_is_provided()
public function test_asset_controller_returns_a_404_when_the_storage_root_doesnt_exist()
{
TenantAssetsController::$tenancyMiddleware = InitializeTenancyByRequestData::class;
$tenant = Tenant::create();
tenancy()->initialize($tenant);
$response = $this->get(tenant_asset('../foo.txt'), [
$storageRoot = storage_path("app/public");
if (is_dir($storageRoot)) {
rmdir(storage_path("app/public"));
}
$this->withoutExceptionHandling();
$this->expectExceptionMessage("Storage root doesn't exist"); // outside tests this is a 404
$this->get(tenant_asset('foo.txt'), [
'X-Tenant' => $tenant->id,
]);
}
$response->assertForbidden();
public function test_asset_controller_returns_a_404_when_accessing_a_nonexistent_file()
{
TenantAssetsController::$tenancyMiddleware = InitializeTenancyByRequestData::class;
$tenant = Tenant::create();
tenancy()->initialize($tenant);
$storageRoot = storage_path("app/public");
if (! is_dir($storageRoot)) {
mkdir(storage_path("app/public"), recursive: true);
}
$this->withoutExceptionHandling();
$this->expectExceptionMessage("Accessing a nonexistent file"); // outside tests this is a 404
$this->get(tenant_asset('foo.txt'), [
'X-Tenant' => $tenant->id,
]);
}
public function test_asset_controller_returns_a_404_when_accessing_a_file_outside_the_storage_root()
{
TenantAssetsController::$tenancyMiddleware = InitializeTenancyByRequestData::class;
$tenant = Tenant::create();
tenancy()->initialize($tenant);
$storageRoot = storage_path("app/public");
if (! is_dir($storageRoot)) {
mkdir(storage_path("app/public"), recursive: true);
file_put_contents(storage_path('app/foo.txt'), 'bar');
}
$this->withoutExceptionHandling();
$this->expectExceptionMessage('Accessing a file outside the storage root'); // outside tests this is a 404
$this->get(tenant_asset('../foo.txt'), [
'X-Tenant' => $tenant->id,
]);
}
}