From 3cad5f56361e0d35485e74db8e813e53c37752b1 Mon Sep 17 00:00:00 2001 From: HolgerHatGarKeineNode <123783602+HolgerHatGarKeineNode@users.noreply.github.com> Date: Mon, 8 Jun 2026 02:53:44 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20**Enhance=20input=20validation=20an?= =?UTF-8?q?d=20error=20handling=20across=20APIs**?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 🛠️ Refactored controllers to utilize `FiltersNumericIds` concern, ensuring secure numeric ID filtering and avoiding type-sensitive errors in queries. - ➕ Added feature tests to validate robust input hardening for non-numeric or malformed query parameters (`user_id`, `selected[]`). - 🔒 Introduced `PublicPropertyNotFoundException` handling in Livewire, returning 400 for invalid property probes and suppressing unnecessary log entries. - ❌ Updated `MeetupEventController` to handle invalid date formats gracefully, aborting with a 400 response instead of 500. - ✅ Expanded exception handling pipeline for enhanced resilience against malformed input, bot noise, and exploitable probes. --- app/Http/Controllers/Api/CityController.php | 6 ++-- .../Api/Concerns/FiltersNumericIds.php | 24 +++++++++++++++ app/Http/Controllers/Api/CourseController.php | 8 +++-- .../Controllers/Api/LecturerController.php | 8 ++--- app/Http/Controllers/Api/MeetupController.php | 5 +++- .../Controllers/Api/MeetupEventController.php | 8 ++++- app/Http/Controllers/Api/VenueController.php | 6 ++-- bootstrap/app.php | 22 ++++++++++++-- .../Api/ApiIndexInputHardeningTest.php | 29 +++++++++++++++++++ tests/Feature/Api/MeetupApiTest.php | 8 +++++ tests/Feature/LivewireExploitProbeTest.php | 23 +++++++++++++++ 11 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 app/Http/Controllers/Api/Concerns/FiltersNumericIds.php create mode 100644 tests/Feature/Api/ApiIndexInputHardeningTest.php diff --git a/app/Http/Controllers/Api/CityController.php b/app/Http/Controllers/Api/CityController.php index 2c1f1e6..b17573e 100644 --- a/app/Http/Controllers/Api/CityController.php +++ b/app/Http/Controllers/Api/CityController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Http\Controllers\Api\Concerns\FiltersNumericIds; use App\Http\Controllers\Controller; use App\Http\Requests\Api\StoreCityRequest; use App\Http\Requests\Api\UpdateCityRequest; @@ -20,6 +21,8 @@ use Symfony\Component\HttpFoundation\Response; #[Group(name: 'Stammdaten', weight: 5)] class CityController extends Controller { + use FiltersNumericIds; + /** * Städte auflisten und durchsuchen * @@ -40,8 +43,7 @@ class CityController extends Controller ) ->when( $request->exists('selected'), - fn (Builder $query) => $query->whereIn('id', - $request->input('selected', [])), + fn (Builder $query) => $query->whereIn('id', $this->numericIds($request)), fn (Builder $query) => $query->limit(10) ) ->get(); diff --git a/app/Http/Controllers/Api/Concerns/FiltersNumericIds.php b/app/Http/Controllers/Api/Concerns/FiltersNumericIds.php new file mode 100644 index 0000000..6340685 --- /dev/null +++ b/app/Http/Controllers/Api/Concerns/FiltersNumericIds.php @@ -0,0 +1,24 @@ + + */ + protected function numericIds(Request $request, string $key = 'selected'): array + { + return $request->collect($key) + ->filter(fn ($id) => is_numeric($id)) + ->map(fn ($id) => (int) $id) + ->values() + ->all(); + } +} diff --git a/app/Http/Controllers/Api/CourseController.php b/app/Http/Controllers/Api/CourseController.php index 1e2b43b..b21c8eb 100644 --- a/app/Http/Controllers/Api/CourseController.php +++ b/app/Http/Controllers/Api/CourseController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Http\Controllers\Api\Concerns\FiltersNumericIds; use App\Http\Controllers\Controller; use App\Models\Course; use Dedoc\Scramble\Attributes\ExcludeRouteFromDocs; @@ -16,6 +17,8 @@ use Symfony\Component\HttpFoundation\Response; #[Group(name: 'Kurse', weight: 1)] class CourseController extends Controller { + use FiltersNumericIds; + /** * Kurse auflisten und durchsuchen * @@ -32,7 +35,7 @@ class CourseController extends Controller ->select('id', 'name') ->orderBy('name') ->when($request->has('user_id'), - fn (Builder $query) => $query->where('created_by', $request->user_id)) + fn (Builder $query) => $query->where('created_by', $request->integer('user_id'))) ->when( $request->search, fn (Builder $query) => $query @@ -40,8 +43,7 @@ class CourseController extends Controller ) ->when( $request->exists('selected'), - fn (Builder $query) => $query->whereIn('id', - $request->input('selected', [])), + fn (Builder $query) => $query->whereIn('id', $this->numericIds($request)), fn (Builder $query) => $query->limit(10) ) ->get() diff --git a/app/Http/Controllers/Api/LecturerController.php b/app/Http/Controllers/Api/LecturerController.php index faac6b4..8ec85bb 100644 --- a/app/Http/Controllers/Api/LecturerController.php +++ b/app/Http/Controllers/Api/LecturerController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Http\Controllers\Api\Concerns\FiltersNumericIds; use App\Http\Controllers\Controller; use App\Http\Requests\Api\StoreLecturerRequest; use App\Http\Requests\Api\UpdateLecturerRequest; @@ -20,6 +21,8 @@ use Symfony\Component\HttpFoundation\Response; #[Group(name: 'Referenten', weight: 4)] class LecturerController extends Controller { + use FiltersNumericIds; + /** * Referenten auflisten und durchsuchen * @@ -32,8 +35,6 @@ class LecturerController extends Controller return Lecturer::query() ->select('id', 'name') ->orderBy('name') -// ->when($request->has('user_id'), -// fn(Builder $query) => $query->where('created_by', $request->user_id)) ->when( $request->search, fn (Builder $query) => $query @@ -41,8 +42,7 @@ class LecturerController extends Controller ) ->when( $request->exists('selected'), - fn (Builder $query) => $query->whereIn('id', - $request->input('selected', [])), + fn (Builder $query) => $query->whereIn('id', $this->numericIds($request)), fn (Builder $query) => $query->limit(10) ) ->get() diff --git a/app/Http/Controllers/Api/MeetupController.php b/app/Http/Controllers/Api/MeetupController.php index 70e51b9..c0dda4b 100644 --- a/app/Http/Controllers/Api/MeetupController.php +++ b/app/Http/Controllers/Api/MeetupController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Http\Controllers\Api\Concerns\FiltersNumericIds; use App\Http\Controllers\Controller; use App\Http\Requests\Api\StoreMeetupRequest; use App\Http\Requests\Api\UpdateMeetupRequest; @@ -20,6 +21,8 @@ use Illuminate\Support\Facades\Gate; #[Group(name: 'Meetups', weight: 3)] class MeetupController extends Controller { + use FiltersNumericIds; + #[ExcludeRouteFromDocs] public function ical() { @@ -58,7 +61,7 @@ class MeetupController extends Controller ) ->when( $request->exists('selected'), - fn (Builder $query) => $query->whereIn('id', $request->input('selected', [])), + fn (Builder $query) => $query->whereIn('id', $this->numericIds($request)), fn (Builder $query) => $query->limit(10), ) ->get() diff --git a/app/Http/Controllers/Api/MeetupEventController.php b/app/Http/Controllers/Api/MeetupEventController.php index b4c88a1..1d05af6 100644 --- a/app/Http/Controllers/Api/MeetupEventController.php +++ b/app/Http/Controllers/Api/MeetupEventController.php @@ -8,6 +8,7 @@ use App\Http\Requests\Api\UpdateMeetupEventRequest; use App\Http\Resources\MeetupEventResource; use App\Models\MeetupEvent; use Carbon\Carbon; +use Carbon\Exceptions\InvalidFormatException; use Dedoc\Scramble\Attributes\Group; use Dedoc\Scramble\Attributes\PathParameter; use Dedoc\Scramble\Attributes\Response as ResponseAttribute; @@ -30,10 +31,15 @@ class MeetupEventController extends Controller * @return Collection> */ #[PathParameter(name: 'date', description: 'Optionales Datum (Y-m-d); filtert auf den Monat dieses Datums.', required: false, type: 'string')] + #[ResponseAttribute(status: 400, description: 'Das übergebene Datum ist nicht parsebar (erwartet wird Y-m-d).')] public function __invoke(?string $date = null): Collection { if ($date) { - $date = Carbon::parse($date); + try { + $date = Carbon::parse($date); + } catch (InvalidFormatException) { + abort(Response::HTTP_BAD_REQUEST, 'Ungültiges Datum. Erwartet wird das Format Y-m-d.'); + } } $events = MeetupEvent::query() ->with([ diff --git a/app/Http/Controllers/Api/VenueController.php b/app/Http/Controllers/Api/VenueController.php index 95ce05c..00ef62b 100644 --- a/app/Http/Controllers/Api/VenueController.php +++ b/app/Http/Controllers/Api/VenueController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Http\Controllers\Api\Concerns\FiltersNumericIds; use App\Http\Controllers\Controller; use App\Http\Requests\Api\StoreVenueRequest; use App\Http\Requests\Api\UpdateVenueRequest; @@ -20,6 +21,8 @@ use Symfony\Component\HttpFoundation\Response; #[Group(name: 'Stammdaten', weight: 5)] class VenueController extends Controller { + use FiltersNumericIds; + /** * Veranstaltungsorte auflisten und durchsuchen * @@ -42,8 +45,7 @@ class VenueController extends Controller ) ->when( $request->exists('selected'), - fn (Builder $query) => $query->whereIn('id', - $request->input('selected', [])), + fn (Builder $query) => $query->whereIn('id', $this->numericIds($request)), fn (Builder $query) => $query->limit(10) ) ->get() diff --git a/bootstrap/app.php b/bootstrap/app.php index 6dcf364..78da8a0 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -8,6 +8,7 @@ use Illuminate\Foundation\Configuration\Exceptions; use Illuminate\Foundation\Configuration\Middleware; use Illuminate\Http\Request; use Livewire\Exceptions\MethodNotFoundException; +use Livewire\Exceptions\PublicPropertyNotFoundException; use Livewire\Features\SupportFileUploads\MissingFileUploadsTraitException; use Livewire\Features\SupportLifecycleHooks\DirectlyCallingLifecycleHooksNotAllowedException; use Livewire\Mechanisms\HandleComponents\CorruptComponentPayloadException; @@ -85,7 +86,16 @@ return Application::configure(basePath: dirname(__DIR__)) return false; }; - $exceptions->report(function (Throwable $e) use ($isStaleLivewireAsset, $isStaleCompiledView, $isMissingFileUploadsTrait, $isLivewireExploitProbe) { + $isMalformedLivewirePropertyUpdate = function (Throwable $e): bool { + // Bots replay `livewire/update` with a mutated snapshot that sets public + // properties the target component never declared (e.g. `value` on `welcome`). + // Livewire rejects the update; in production this is pure bot noise, so we + // silence it. Locally we let it surface, so a genuinely undeclared property + // binding is still caught during development. + return $e instanceof PublicPropertyNotFoundException && ! app()->isLocal(); + }; + + $exceptions->report(function (Throwable $e) use ($isStaleLivewireAsset, $isStaleCompiledView, $isMissingFileUploadsTrait, $isLivewireExploitProbe, $isMalformedLivewirePropertyUpdate) { if ($isStaleLivewireAsset($e, request())) { return false; } @@ -102,6 +112,10 @@ return Application::configure(basePath: dirname(__DIR__)) return false; } + if ($isMalformedLivewirePropertyUpdate($e)) { + return false; + } + // Bots replay `/livewire/update` with a mutated snapshot whose HMAC // checksum no longer matches its [name, id, data]. Checksum::verify() // rejects these, so the rejection is the tamper signature, not an app @@ -114,7 +128,7 @@ return Application::configure(basePath: dirname(__DIR__)) return null; }); - $exceptions->render(function (Throwable $e, Request $request) use ($isStaleLivewireAsset, $isStaleCompiledView, $isMissingFileUploadsTrait, $isLivewireExploitProbe) { + $exceptions->render(function (Throwable $e, Request $request) use ($isStaleLivewireAsset, $isStaleCompiledView, $isMissingFileUploadsTrait, $isLivewireExploitProbe, $isMalformedLivewirePropertyUpdate) { if ($isStaleLivewireAsset($e, $request)) { return response('', 404); } @@ -131,6 +145,10 @@ return Application::configure(basePath: dirname(__DIR__)) return response('', 400); } + if ($isMalformedLivewirePropertyUpdate($e)) { + return response('', 400); + } + return null; }); })->create(); diff --git a/tests/Feature/Api/ApiIndexInputHardeningTest.php b/tests/Feature/Api/ApiIndexInputHardeningTest.php new file mode 100644 index 0000000..b4b850d --- /dev/null +++ b/tests/Feature/Api/ApiIndexInputHardeningTest.php @@ -0,0 +1,29 @@ +create(); + + $response = $this->getJson('/api/courses?selected[]='.$course->id.'&selected[]=foo'); + + $response->assertSuccessful(); + expect(collect($response->json())->pluck('id')->all())->toBe([$course->id]); +}); + +it('casts a non-numeric user_id to an empty filter on GET /api/courses', function () { + Course::factory()->create(); + + $this->getJson('/api/courses?user_id=abc') + ->assertSuccessful() + ->assertJsonCount(0); +}); + +it('tolerates a non-array selected value on GET /api/venues without a 500', function () { + Venue::factory()->create(); + + $this->getJson('/api/venues?selected=foo') + ->assertSuccessful() + ->assertJsonCount(0); +}); diff --git a/tests/Feature/Api/MeetupApiTest.php b/tests/Feature/Api/MeetupApiTest.php index 179dc16..8bb751c 100644 --- a/tests/Feature/Api/MeetupApiTest.php +++ b/tests/Feature/Api/MeetupApiTest.php @@ -94,3 +94,11 @@ it('filters /api/meetup-events by date when one is supplied', function () { $response->assertSuccessful(); expect($response->json())->toBeArray()->not->toBeEmpty(); }); + +it('returns 400 instead of 500 when the date path segment is not parseable', function () { + $this->getJson('/api/meetup-events/'.urlencode('{date}')) + ->assertStatus(400); + + $this->getJson('/api/meetup-events/not-a-date') + ->assertStatus(400); +}); diff --git a/tests/Feature/LivewireExploitProbeTest.php b/tests/Feature/LivewireExploitProbeTest.php index 10ae6a1..4ef4c2a 100644 --- a/tests/Feature/LivewireExploitProbeTest.php +++ b/tests/Feature/LivewireExploitProbeTest.php @@ -3,6 +3,7 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Route; use Livewire\Exceptions\MethodNotFoundException; +use Livewire\Exceptions\PublicPropertyNotFoundException; use Livewire\Features\SupportLifecycleHooks\DirectlyCallingLifecycleHooksNotAllowedException; use Livewire\Mechanisms\HandleComponents\CorruptComponentPayloadException; @@ -44,6 +45,28 @@ it('still surfaces genuine method-not-found bugs', function () { expect($this->get('/_test/livewire-real-method-not-found')->status())->not->toBe(400); }); +it('returns 400 for setting an undeclared public property instead of 500', function () { + Route::get('/_test/livewire-undeclared-property', function () { + throw new PublicPropertyNotFoundException('value', 'welcome'); + }); + + expect($this->get('/_test/livewire-undeclared-property')->status())->toBe(400); +}); + +it('does not report undeclared-property probes to the logs', function () { + Log::spy(); + + Route::get('/_test/livewire-undeclared-property-log', function () { + throw new PublicPropertyNotFoundException('value', 'welcome'); + }); + + $this->get('/_test/livewire-undeclared-property-log')->assertStatus(400); + + Log::shouldNotHaveReceived('error'); + Log::shouldNotHaveReceived('critical'); + Log::shouldNotHaveReceived('emergency'); +}); + it('does not report corrupt Livewire snapshot payloads', function () { Log::spy();