Profilfoto: Upload nach Inhalt pruefen, Loeschen nur per POST
- Slim::storeCroppedImage() vertraute dem vom Client gemeldeten Typ und uebernahm die Endung aus dem Dateinamen; "shell.php" mit Typ image/png landete als .php im public-Disk. Jetzt entscheidet getimagesizefromstring() ueber die Bytes, die Endung kommt aus dem geprueften Typ. - Foto loeschen war ein GET-Link und liess sich per fremdem <img src> ausloesen (CSRF); jetzt POST-Formular mit Token. - Ohne Konto (users.account_id) bricht Upload/Loeschen mit 404 ab statt mit einem Fatal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
22dcc0f972
commit
e009b11259
5 changed files with 90 additions and 14 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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)) {
|
||||
|
|
|
|||
|
|
@ -14,9 +14,15 @@
|
|||
'size' => 120,
|
||||
])
|
||||
<br><br>
|
||||
<a href="{{ route('user_profile_delete_image') }}" class="btn btn-danger"
|
||||
onclick="return confirm('{{ __('actions.really_delete_picture') }}');"><i class="fa fa-trash"></i>
|
||||
{{ __('account.profile_image_delete') }}</a>
|
||||
{{-- Loeschen per POST: als GET-Link liesse sich das Foto ueber ein
|
||||
fremdes <img src> loeschen (CSRF). --}}
|
||||
<form method="POST" action="{{ route('user_profile_delete_image') }}" accept-charset="UTF-8"
|
||||
class="d-inline"
|
||||
onsubmit="return confirm('{{ __('actions.really_delete_picture') }}');">
|
||||
@csrf
|
||||
<button type="submit" class="btn btn-danger"><i class="fa fa-trash"></i>
|
||||
{{ __('account.profile_image_delete') }}</button>
|
||||
</form>
|
||||
</div>
|
||||
@else
|
||||
<form method="POST" action="{{ route('user_profile_upload_image') }}" accept-charset="UTF-8" class="avatar"
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ Route::domain(config('app.pre_url_crm').config('app.domain').config('app.tld_car
|
|||
|
||||
// Berater-Profilfoto (N6)
|
||||
Route::post('/user/profile/upload/image', 'ProfileImageController@uploadImage')->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');
|
||||
|
|
|
|||
|
|
@ -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' => '<?php echo 1;', 'name' => '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');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue