diff --git a/app/Http/Controllers/ProfileImageController.php b/app/Http/Controllers/ProfileImageController.php index ff5523e6..76bf22b9 100644 --- a/app/Http/Controllers/ProfileImageController.php +++ b/app/Http/Controllers/ProfileImageController.php @@ -21,6 +21,12 @@ class ProfileImageController extends Controller { $user = Auth::user(); + // Das Foto haengt am Konto (users.account_id). Ohne Konto gibt es nichts + // zu speichern — sonst liefe der Zugriff darunter in einen Fatal. + if (! $user->account) { + abort(404); + } + if (! Request::get('profile_image_consent')) { \Session()->flash('alert-danger', __('account.profile_image_consent_required')); @@ -66,7 +72,7 @@ class ProfileImageController extends Controller { $user = Auth::user(); - if ($user->account->profile_image) { + if ($user->account?->profile_image) { \Storage::disk('public')->delete('images/profile/'.$user->account->profile_image); $user->account->profile_image = null; diff --git a/app/Services/Slim.php b/app/Services/Slim.php index 33c3d8c4..902828ad 100644 --- a/app/Services/Slim.php +++ b/app/Services/Slim.php @@ -17,7 +17,17 @@ class Slim /** * Speichert ein Slim-Cropper-Upload (Base64-Payload aus getImages()) im * public-Disk und liefert Dateiname + Metadaten — gemeinsame Logik für - * Shop- und Profilfoto-Upload. Null bei nicht erlaubtem Dateityp. + * Shop- und Profilfoto-Upload. Null, wenn der Inhalt kein JPEG/PNG ist. + * + * Zwei Dinge sind hier sicherheitsrelevant und dürfen nicht vereinfacht + * werden: + * + * 1. `output.type` ist eine ANGABE DES CLIENTS aus dem geposteten JSON, kein + * geprüfter Inhaltstyp. Massgeblich ist deshalb, was + * getimagesizefromstring() in den Bytes findet. + * 2. Die Endung kommt IMMER aus dem geprüften Typ, nie aus dem übermittelten + * Dateinamen: {@see sanitizeFileName()} lässt jede Endung stehen, ein + * "foto.php" landete sonst als ausführbarer Dateiname im public-Disk. * * @param array $image Ein Eintrag aus {@see getImages()} * @return array{filename: string, originalname: string, ext: string, mime: string, size: mixed}|null @@ -26,19 +36,31 @@ class Slim { $allowed = ['image/jpeg' => 'jpg', 'image/png' => 'png']; - if (! isset($allowed[$image['output']['type']])) { + $declaredType = $image['output']['type'] ?? null; + $data = $image['output']['data'] ?? null; + + if (! is_string($declaredType) || ! isset($allowed[$declaredType]) || ! is_string($data) || $data === '') { return null; } - $name = uniqid().'_'.Slim::sanitizeFileName($image['output']['name']); + $info = @getimagesizefromstring($data); + $mime = is_array($info) ? ($info['mime'] ?? null) : null; - \Storage::disk('public')->put(rtrim($directory, '/').'/'.$name, $image['output']['data']); + if (! is_string($mime) || ! isset($allowed[$mime])) { + return null; + } + + $ext = $allowed[$mime]; + $base = Slim::sanitizeFileName(pathinfo((string) ($image['output']['name'] ?? ''), PATHINFO_FILENAME)); + $name = uniqid().'_'.$base.'.'.$ext; + + \Storage::disk('public')->put(rtrim($directory, '/').'/'.$name, $data); return [ 'filename' => $name, - 'originalname' => $image['output']['name'], - 'ext' => $allowed[$image['output']['type']], - 'mime' => $image['output']['type'], + 'originalname' => $image['output']['name'] ?? '', + 'ext' => $ext, + 'mime' => $mime, 'size' => $image['input']['size'] ?? null, ]; } @@ -118,7 +140,7 @@ class Slim if (isset($data->output)) { - $outputDate = null; + $outputData = null; if (isset($data->output->image)) { $outputData = Slim::getBase64Data($data->output->image); } elseif (isset($data->output->field)) { diff --git a/resources/views/user/components/user_profile_image.blade.php b/resources/views/user/components/user_profile_image.blade.php index 98a27996..6ba2dcc7 100644 --- a/resources/views/user/components/user_profile_image.blade.php +++ b/resources/views/user/components/user_profile_image.blade.php @@ -14,9 +14,15 @@ 'size' => 120, ])

- - {{ __('account.profile_image_delete') }} + {{-- Loeschen per POST: als GET-Link liesse sich das Foto ueber ein + fremdes loeschen (CSRF). --}} +
+ @csrf + +
@else
name('user_profile_upload_image'); - Route::get('/user/profile/delete/image', 'ProfileImageController@deleteImage')->name('user_profile_delete_image'); + Route::post('/user/profile/delete/image', 'ProfileImageController@deleteImage')->name('user_profile_delete_image'); Route::get('/user/update_password', 'UserUpdatePasswordController@updatePassword')->name('user_update_password'); Route::post('/user/update_password', 'UserUpdatePasswordController@updatePasswordStore')->name('user_update_password'); diff --git a/tests/Feature/ProfileImageTest.php b/tests/Feature/ProfileImageTest.php index 416a1d22..9ca3e9fe 100644 --- a/tests/Feature/ProfileImageTest.php +++ b/tests/Feature/ProfileImageTest.php @@ -29,6 +29,16 @@ function piMakeUser(): User ]); } +/** Echte Bytes eines 1x1-JPEG bzw. -PNG — storeCroppedImage() prueft den Inhalt, nicht die Client-Angabe. */ +function piImageBytes(string $type): string +{ + $base64 = $type === 'png' + ? 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' + : '/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhCY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAARCAABAAEDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAAAgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkKFhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWGh4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl5ufo6ery8/T19vf4+fr/xAAfAQADAQEBAQEBAQEBAAAAAAAAAQIDBAUGBwgJCgv/xAC1EQACAQIEBAMEBwUEBAABAncAAQIDEQQFITEGEkFRB2FxEyIygQgUQpGhscEJIzNS8BVictEKFiQ04SXxFxgZGiYnKCkqNTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqCg4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2dri4+Tl5ufo6ery8/T19vf4+fr/2gAMAwEAAhEDEQA/APP6KKKAP//Z'; + + return base64_decode($base64); +} + it('verweigert den Foto-Upload ohne Einwilligung', function (): void { URL::resolveMissingNamedRoutesUsing(fn (string $name, $parameters = []) => '/'.$name); Storage::fake('public'); @@ -48,7 +58,7 @@ it('speichert ein Slim-Upload nur fuer jpg/png und liefert die Metadaten', funct $image = [ 'input' => ['size' => 1234], - 'output' => ['data' => 'BINARYDATA', 'name' => 'Mein Foto!.jpg', 'type' => 'image/jpeg'], + 'output' => ['data' => piImageBytes('jpg'), 'name' => 'Mein Foto!.jpg', 'type' => 'image/jpeg'], ]; $stored = Slim::storeCroppedImage($image, 'images/profile'); @@ -63,6 +73,38 @@ it('speichert ein Slim-Upload nur fuer jpg/png und liefert die Metadaten', funct expect(Slim::storeCroppedImage($gif, 'images/profile'))->toBeNull(); }); +it('legt die Datei-Endung nach dem geprueften Inhalt an, nicht nach dem Dateinamen', function (): void { + // Angriffsbild: der Client deklariert image/png, nennt die Datei aber .php. + // Frueher landete genau dieser Name im oeffentlichen Disk. + Storage::fake('public'); + + $image = [ + 'input' => ['size' => 10], + 'output' => ['data' => piImageBytes('png'), 'name' => 'shell.php', 'type' => 'image/png'], + ]; + + $stored = Slim::storeCroppedImage($image, 'images/profile'); + + expect($stored)->not->toBeNull() + ->and($stored['filename'])->toEndWith('.png') + ->and($stored['filename'])->not->toContain('.php'); + Storage::disk('public')->assertExists('images/profile/'.$stored['filename']); +}); + +it('lehnt einen Upload ab, dessen Inhalt kein Bild ist', function (): void { + // Die Typ-Angabe kommt aus dem geposteten JSON und ist damit frei waehlbar — + // massgeblich sind die Bytes. + Storage::fake('public'); + + $image = [ + 'input' => ['size' => 10], + 'output' => ['data' => ' 'foto.png', 'type' => 'image/png'], + ]; + + expect(Slim::storeCroppedImage($image, 'images/profile'))->toBeNull(); + expect(Storage::disk('public')->allFiles('images/profile'))->toBe([]); +}); + it('loescht das Profilfoto samt Datei', function (): void { URL::resolveMissingNamedRoutesUsing(fn (string $name, $parameters = []) => '/'.$name); Storage::fake('public');