From 6578335568a7c42f5db45b6ce68099d4c7bca39f Mon Sep 17 00:00:00 2001 From: Andreas Reinhold / reini Date: Thu, 8 Oct 2026 23:23:00 +0200 Subject: [PATCH] Fix the security audit's findings ShareDownload::$authenticated was a public property without #[Locked], so a visitor with a share's link but not its password could set it from the browser and see the file list: names, folder paths, sizes and whether the share holds a text. The contents stayed protected; the flag is now locked, and a test tries to set it. Sessions hold the keys of the password shares a recipient unlocked, and were stored unencrypted in the sessions table wherever SESSION_ENCRYPT was not set, which includes every Docker install. They are now encrypted unless SESSION_ENCRYPT=false. Share keys never live in the session, so every share stays readable; people sign in again once. The container generated an APP_KEY when none was set, printed it to the logs and generated another on the next start, which left the shares without a password unreadable. It now refuses to start without one and says how to create it; docker-compose.example.yml requires it for both services. The command it names, and the README's, now pass --entrypoint php: the image's entrypoint ignores its arguments, so "docker run php artisan key:generate --show" never ran it. Every proxy is trusted, so the password-reset mail built its link from a host a forged X-Forwarded-Host could choose. It is now built from APP_URL. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- app/Livewire/ShareDownload.php | 3 +++ app/Providers/FortifyServiceProvider.php | 8 ++++++++ config/session.php | 10 ++++++++-- docker-compose.example.yml | 11 ++++++----- docker/entrypoint.sh | 13 +++++++------ tests/Feature/Auth/PasswordResetTest.php | 14 ++++++++++++++ tests/Feature/SecurityAuditTest.php | 17 +++++++++++++++++ 8 files changed, 64 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 89eefbd..cdf4be6 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ curl -O https://gitea.nonameweb.ch/noNameWEB/SealShare/raw/branch/main/docker-co cp docker-compose.example.yml docker-compose.yml # Generate an app key and paste it into docker-compose.yml -docker run --rm gitea.nonameweb.ch/nonameweb/sealshare:latest php artisan key:generate --show +docker run --rm --entrypoint php gitea.nonameweb.ch/nonameweb/sealshare:latest artisan key:generate --show # Edit docker-compose.yml — set APP_KEY and APP_URL, and choose how HTTPS is served (below) # Then start: diff --git a/app/Livewire/ShareDownload.php b/app/Livewire/ShareDownload.php index 1e28539..8242797 100644 --- a/app/Livewire/ShareDownload.php +++ b/app/Livewire/ShareDownload.php @@ -7,6 +7,7 @@ use App\Services\ShareService; use Carbon\CarbonInterval; use Illuminate\Support\Facades\RateLimiter; use Livewire\Attributes\Layout; +use Livewire\Attributes\Locked; use Livewire\Attributes\Renderless; use Livewire\Attributes\Validate; use Livewire\Component; @@ -16,6 +17,8 @@ class ShareDownload extends Component { public Share $share; + /** Set only by mount() and verifyPassword(): the browser must never unlock the file list. */ + #[Locked] public bool $authenticated = false; #[Validate('required|string')] diff --git a/app/Providers/FortifyServiceProvider.php b/app/Providers/FortifyServiceProvider.php index 4bb9d0e..d8bd84a 100644 --- a/app/Providers/FortifyServiceProvider.php +++ b/app/Providers/FortifyServiceProvider.php @@ -3,6 +3,8 @@ namespace App\Providers; use App\Actions\Fortify\ResetUserPassword; +use App\Models\User; +use Illuminate\Auth\Notifications\ResetPassword; use Illuminate\Cache\RateLimiting\Limit; use Illuminate\Http\Request; use Illuminate\Support\Facades\RateLimiter; @@ -28,6 +30,12 @@ class FortifyServiceProvider extends ServiceProvider private function configureActions(): void { Fortify::resetUserPasswordsUsing(ResetUserPassword::class); + + // From APP_URL, never the request's host: every proxy is trusted, so a forged X-Forwarded-Host would otherwise pick it. + ResetPassword::createUrlUsing(fn (User $user, string $token): string => rtrim((string) config('app.url'), '/').route('password.reset', [ + 'token' => $token, + 'email' => $user->getEmailForPasswordReset(), + ], false)); } /** diff --git a/config/session.php b/config/session.php index 9f47b35..4b5a5dc 100644 --- a/config/session.php +++ b/config/session.php @@ -8,13 +8,19 @@ use Illuminate\Support\Str; |-------------------------------------------------------------------------- | | Kept from earlier releases, where it differs from the framework's -| "_session": renaming the cookie would sign everyone out. Every -| other key comes from the framework's defaults. +| "_session": renaming the cookie would sign everyone out. +| +| Sessions are encrypted unless SESSION_ENCRYPT=false: they hold the keys +| of the password shares a recipient unlocked, which must never sit +| readable in the sessions table. Every other key comes from the +| framework's defaults. | */ return [ + 'encrypt' => env('SESSION_ENCRYPT', true), + 'cookie' => env( 'SESSION_COOKIE', Str::slug((string) env('APP_NAME', 'laravel')).'-session' diff --git a/docker-compose.example.yml b/docker-compose.example.yml index 985c6bd..62a4152 100644 --- a/docker-compose.example.yml +++ b/docker-compose.example.yml @@ -4,12 +4,13 @@ # # Quick start: # 1. Copy this file: cp docker-compose.example.yml docker-compose.yml -# 2. Edit the settings below (APP_URL is required; uploads need HTTPS, see below) +# 2. Edit the settings below (APP_URL and APP_KEY are required; uploads need HTTPS, see below) # 3. Start: docker compose up -d # 4. Open your browser to your configured domain # -# Note: APP_KEY is auto-generated on first start if not set. -# Copy it from the logs into your docker-compose.yml to persist across restarts. +# APP_KEY is required: the container does not start without it. Create one with +# docker run --rm --entrypoint php gitea.nonameweb.ch/nonameweb/sealshare:latest artisan key:generate --show +# and keep it: it decrypts the shares, so a new key makes the existing ones unreadable. # # ============================================ @@ -32,7 +33,7 @@ services: environment: # --- REQUIRED --- APP_URL: # Your full URL, e.g. https://share.example.com - # APP_KEY: # Auto-generated if not set. Copy from logs to persist across restarts. + APP_KEY: # Required. Create one with php artisan key:generate --show (see above) DB_DATABASE: /app/database/sqlite/database.sqlite # The SQLite file in sealshare_database # --- HTTPS --- @@ -90,7 +91,7 @@ services: - sealshare_storage:/app/storage/app - sealshare_database:/app/database/sqlite environment: - # APP_KEY: # Same key as the app service above (auto-generated if not set) + APP_KEY: # Same key as the app service above APP_URL: # Same URL as the app service above DB_DATABASE: /app/database/sqlite/database.sqlite # Same as the app service above depends_on: diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 19ee1d7..c4f7a93 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -3,13 +3,14 @@ set -e cd /app -# Auto-generate APP_KEY if not provided +# APP_KEY decrypts the shares, so it must stay the same across restarts: a key made up here would +# change on the next start and leave every existing share unreadable. if [ -z "$APP_KEY" ]; then - echo "[entrypoint] No APP_KEY set, generating one..." - APP_KEY=$(php artisan key:generate --show) - export APP_KEY - echo "[entrypoint] Generated APP_KEY: $APP_KEY" - echo "[entrypoint] WARNING: Set this APP_KEY in your docker-compose.yml to persist across restarts!" + echo "[entrypoint] APP_KEY is not set. Create one and set it for the app and the scheduler:" >&2 + echo "[entrypoint] docker run --rm --entrypoint php artisan key:generate --show" >&2 + echo "[entrypoint] If this install ran without one before, use the key from its old logs" >&2 + echo "[entrypoint] (\"Generated APP_KEY: ...\") so its shares stay readable." >&2 + exit 1 fi # A docker-compose.yml from before 2.1.1 mounts the SQLite volume over all of /app/database, so the diff --git a/tests/Feature/Auth/PasswordResetTest.php b/tests/Feature/Auth/PasswordResetTest.php index bc8a005..c7ffeb2 100644 --- a/tests/Feature/Auth/PasswordResetTest.php +++ b/tests/Feature/Auth/PasswordResetTest.php @@ -20,6 +20,20 @@ test('reset password link can be requested', function () { Notification::assertSentTo($user, ResetPassword::class); }); +test('the reset link points at APP_URL, whatever host the request claims', function () { + Notification::fake(); + config(['app.url' => 'https://share.example.com']); + + $user = User::factory()->create(); + + $this->withHeader('X-Forwarded-Host', 'attacker.example') + ->post(route('password.request'), ['email' => $user->email]); + + Notification::assertSentTo($user, ResetPassword::class, function (ResetPassword $notification) use ($user) { + return str_starts_with($notification->toMail($user)->actionUrl, 'https://share.example.com/reset-password/'); + }); +}); + test('reset password screen can be rendered', function () { Notification::fake(); diff --git a/tests/Feature/SecurityAuditTest.php b/tests/Feature/SecurityAuditTest.php index 56dfebe..596f550 100644 --- a/tests/Feature/SecurityAuditTest.php +++ b/tests/Feature/SecurityAuditTest.php @@ -10,6 +10,7 @@ use App\Services\ShareService; use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Storage; +use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException; use Livewire\Livewire; // --- Session stores derived key, not plaintext password --- @@ -35,6 +36,22 @@ test('session stores derived encryption key instead of plaintext password', func expect(strlen(session('share_key_'.$share->token)))->toBe(64); }); +// --- The password gate cannot be opened from the browser --- + +test('a visitor without the password cannot unlock a share\'s file list from the browser', function () { + Storage::fake('shares'); + + $share = app(ShareService::class)->createShare([ + ['file' => UploadedFile::fake()->createWithContent('salary-review.pdf', 'secret'), 'relativePath' => null], + ], [ + 'password' => 'test-password-secure', + ]); + + Livewire::test(ShareDownload::class, ['share' => $share]) + ->assertDontSee('salary-review.pdf') + ->set('authenticated', true); +})->throws(CannotUpdateLockedPropertyException::class); + // --- Rate limiting on password verification --- test('rate limiting blocks after 5 failed password attempts', function () {