Fix the security audit's findings
Fixes the nine findings of security audit run 1 on 2.4.1 (T1-T11). F1: Uploads that are not complete now count only the bytes the server received, and a chunk that goes above the storage quota gets 422. An anonymous uploader can no longer fill the quota with declared sizes. F2: The share password and the admin login have a second limit without the address (20 attempts per minute per share and per e-mail), because a forged X-Forwarded-For changes the address. F3: The system password gate and the admin check now also run on Livewire updates, so an old uploader snapshot cannot register files. F4: The session keeps a fingerprint of the system password's hash. When the admin changes the system password, earlier verifications end. F5: The page after the upload opens only in the session that created the share, and only while recipients can still reach the share. F6: The share password limit counts an attempt before the password is checked, so parallel attempts cannot pass it. F7: The system password has the same two limits (5 per address, 20 in total per minute). F8: A password change or reset signs the user's other sessions out. F9: The only admin cannot delete the account, which would open /setup to every visitor. The setup wizard also holds the admin password to the password defaults, and Livewire applies EnsureAdmin to updates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,11 +3,14 @@
|
||||
use App\Livewire\Admin\AdminSettings;
|
||||
use App\Livewire\FileUploader;
|
||||
use App\Livewire\ShareDownload;
|
||||
use App\Models\Setting;
|
||||
use App\Models\Share;
|
||||
use App\Models\ShareFile;
|
||||
use App\Models\User;
|
||||
use App\Services\ShareService;
|
||||
use Illuminate\Contracts\Hashing\Hasher;
|
||||
use Illuminate\Http\UploadedFile;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
use Illuminate\Support\Facades\RateLimiter;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException;
|
||||
@@ -106,6 +109,101 @@ test('rate limiter clears after successful password verification', function () {
|
||||
expect(RateLimiter::remaining($rateLimitKey, 5))->toBe(5);
|
||||
});
|
||||
|
||||
test('a share password attempt is counted before the password is checked', function () {
|
||||
Storage::fake('shares');
|
||||
|
||||
$share = app(ShareService::class)->createShare([
|
||||
['file' => UploadedFile::fake()->create('file.txt', 100), 'relativePath' => null],
|
||||
], [
|
||||
'password' => 'correct-password',
|
||||
]);
|
||||
|
||||
$ipKey = 'share-password:'.$share->token.'|127.0.0.1';
|
||||
$attemptsSeenDuringCheck = new ArrayObject;
|
||||
|
||||
Hash::swap(new class(Hash::getFacadeRoot(), $ipKey, $attemptsSeenDuringCheck) implements Hasher
|
||||
{
|
||||
public function __construct(private Hasher $inner, private string $ipKey, private ArrayObject $seen) {}
|
||||
|
||||
public function info($hashedValue): array
|
||||
{
|
||||
return $this->inner->info($hashedValue);
|
||||
}
|
||||
|
||||
public function make(#[SensitiveParameter] $value, array $options = []): string
|
||||
{
|
||||
return $this->inner->make($value, $options);
|
||||
}
|
||||
|
||||
public function check(#[SensitiveParameter] $value, $hashedValue, array $options = []): bool
|
||||
{
|
||||
$this->seen[] = RateLimiter::attempts($this->ipKey);
|
||||
|
||||
return $this->inner->check($value, $hashedValue, $options);
|
||||
}
|
||||
|
||||
public function needsRehash($hashedValue, array $options = []): bool
|
||||
{
|
||||
return false;
|
||||
}
|
||||
});
|
||||
|
||||
Livewire::test(ShareDownload::class, ['share' => $share])
|
||||
->set('password', 'wrong-password')
|
||||
->call('verifyPassword');
|
||||
|
||||
expect($attemptsSeenDuringCheck->getArrayCopy())->not->toBeEmpty()
|
||||
->each->toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
test('the share-wide limit refuses the correct password from any address', function () {
|
||||
Storage::fake('shares');
|
||||
|
||||
$share = app(ShareService::class)->createShare([
|
||||
['file' => UploadedFile::fake()->create('file.txt', 100), 'relativePath' => null],
|
||||
], [
|
||||
'password' => 'correct-password',
|
||||
]);
|
||||
|
||||
for ($i = 0; $i < 20; $i++) {
|
||||
RateLimiter::hit('share-password:'.$share->token, 60);
|
||||
}
|
||||
|
||||
Livewire::test(ShareDownload::class, ['share' => $share])
|
||||
->set('password', 'correct-password')
|
||||
->call('verifyPassword')
|
||||
->assertHasErrors(['password'])
|
||||
->assertSee('Too many attempts')
|
||||
->assertSet('authenticated', false);
|
||||
});
|
||||
|
||||
// --- The system password gate holds for Livewire updates ---
|
||||
|
||||
test('a file uploader snapshot from before the system password was set cannot register files in an unverified session', function () {
|
||||
Storage::fake('shares');
|
||||
|
||||
preg_match_all('/wire:snapshot="([^"]+)"/', $this->get(route('upload'))->assertOk()->getContent(), $matches);
|
||||
$snapshot = collect($matches[1])
|
||||
->map(fn (string $raw): array => json_decode(htmlspecialchars_decode($raw, ENT_QUOTES), true))
|
||||
->first(fn (array $snapshot): bool => str_contains($snapshot['memo']['name'] ?? '', 'file-uploader'));
|
||||
expect($snapshot)->not->toBeNull();
|
||||
|
||||
Setting::set('system_password', Hash::make('system-secret'));
|
||||
$this->flushSession();
|
||||
|
||||
$response = $this->withHeaders(['X-Livewire' => '1'])
|
||||
->postJson(app('livewire')->getUpdateUri(), [
|
||||
'components' => [[
|
||||
'snapshot' => json_encode($snapshot),
|
||||
'updates' => [],
|
||||
'calls' => [['method' => 'registerFiles', 'params' => [[['name' => 'file.bin', 'size' => 1024]]], 'path' => '']],
|
||||
]],
|
||||
]);
|
||||
|
||||
$response->assertRedirect(route('system-password'));
|
||||
expect(Share::query()->count())->toBe(0);
|
||||
});
|
||||
|
||||
// --- Share password minimum length ---
|
||||
|
||||
test('share password must be at least 8 characters', function () {
|
||||
|
||||
Reference in New Issue
Block a user