diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 6870a17c..2aab2732 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,7 +1,7 @@ blank_issues_enabled: false contact_links: - name: Support Questions & Other - url: https://github.com/stancl/tenancy/blob/3.x/SUPPORT.md + url: https://archte.ch/discord about: 'If you have a question or need help using the package.' - name: Documentation Issue url: https://github.com/stancl/tenancy-docs/issues diff --git a/.gitignore b/.gitignore index f470ba75..c7cf933c 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ coverage/ clover.xml tests/Etc/tmp/queuetest.json docker-compose.override.yml +.DS_Store diff --git a/src/Controllers/TenantAssetsController.php b/src/Controllers/TenantAssetsController.php index 5549da0d..356c25dd 100644 --- a/src/Controllers/TenantAssetsController.php +++ b/src/Controllers/TenantAssetsController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Controllers; +use Exception; use Illuminate\Routing\Controller; use Throwable; @@ -18,7 +19,7 @@ class TenantAssetsController extends Controller public function asset($path = null) { - abort_if($path === null, 404); + $this->validatePath($path); try { return response()->file(storage_path("app/public/$path")); @@ -26,4 +27,43 @@ class TenantAssetsController extends Controller abort(404); } } + + /** + * 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 + { + $this->abortIf($path === null, 'Empty path'); + + $allowedRoot = realpath(storage_path('app/public')); + + // `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); + } + } + } } diff --git a/src/Middleware/InitializeTenancyByRequestData.php b/src/Middleware/InitializeTenancyByRequestData.php index de75d8c5..ef2132e8 100644 --- a/src/Middleware/InitializeTenancyByRequestData.php +++ b/src/Middleware/InitializeTenancyByRequestData.php @@ -6,6 +6,7 @@ namespace Stancl\Tenancy\Middleware; use Closure; use Illuminate\Http\Request; +use Stancl\Tenancy\Contracts\TenantResolver; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use Stancl\Tenancy\Tenancy; diff --git a/tests/TenantAssetTest.php b/tests/TenantAssetTest.php index 703ac65e..2852eae2 100644 --- a/tests/TenantAssetTest.php +++ b/tests/TenantAssetTest.php @@ -134,11 +134,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_404_when_the_storage_root_doesnt_exist() + { + TenantAssetsController::$tenancyMiddleware = InitializeTenancyByRequestData::class; + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $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, + ]); + } + + 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, + ]); + } }