From 6312c18a910407eab5c8e796df483172e0262f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tollk=C3=BChn?= Date: Thu, 22 Oct 2026 19:33:03 +0200 Subject: [PATCH] Harden invitee input and contact names Security audit found two gaps. Sabre only quotes ICS parameter values containing space, newline, quote, colon, semicolon, caret, comma or plus, so a bare carriage return in a contact name would be written into the serialized event unquoted and break the line structure; names with control characters are now dropped. The raw attendee input and the search query had no length bound, a huge POST field was fully split, iterated and echoed back on re-render, a cheap CPU sink; both are now capped, the query at the longest possible email fragment. --- l10n/de.json | 3 +- l10n/de_DE.json | 3 +- l10n/en.json | 3 +- lib/Controller/CalendarController.php | 30 ++++++++++++++++- tests/Unit/CalendarControllerTest.php | 48 +++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 4 deletions(-) diff --git a/l10n/de.json b/l10n/de.json index 3b56935..a93e342 100644 --- a/l10n/de.json +++ b/l10n/de.json @@ -34,7 +34,8 @@ "Invitees": "Teilnehmer", "Email addresses, comma-separated": "E-Mail-Adressen, durch Kommas getrennt", "%s is not a valid email address.": "%s ist keine gültige E-Mail-Adresse.", - "A meeting can invite at most %d people.": "Ein Meeting kann höchstens %d Personen einladen." + "A meeting can invite at most %d people.": "Ein Meeting kann höchstens %d Personen einladen.", + "Invitee list is too long.": "Die Teilnehmerliste ist zu lang." }, "pluralForms": "nplurals=2; plural=(n != 1);" } diff --git a/l10n/de_DE.json b/l10n/de_DE.json index 0ad7ce6..cb08cb2 100644 --- a/l10n/de_DE.json +++ b/l10n/de_DE.json @@ -34,7 +34,8 @@ "Invitees": "Teilnehmer", "Email addresses, comma-separated": "E-Mail-Adressen, durch Kommas getrennt", "%s is not a valid email address.": "%s ist keine gültige E-Mail-Adresse.", - "A meeting can invite at most %d people.": "Ein Meeting kann höchstens %d Personen einladen." + "A meeting can invite at most %d people.": "Ein Meeting kann höchstens %d Personen einladen.", + "Invitee list is too long.": "Die Teilnehmerliste ist zu lang." }, "pluralForms": "nplurals=2; plural=(n != 1);" } diff --git a/l10n/en.json b/l10n/en.json index ff099f6..868e775 100644 --- a/l10n/en.json +++ b/l10n/en.json @@ -34,7 +34,8 @@ "Invitees": "Invitees", "Email addresses, comma-separated": "Email addresses, comma-separated", "%s is not a valid email address.": "%s is not a valid email address.", - "A meeting can invite at most %d people.": "A meeting can invite at most %d people." + "A meeting can invite at most %d people.": "A meeting can invite at most %d people.", + "Invitee list is too long.": "Invitee list is too long." }, "pluralForms": "nplurals=2; plural=(n != 1);" } diff --git a/lib/Controller/CalendarController.php b/lib/Controller/CalendarController.php index d4ad5f9..20119a4 100644 --- a/lib/Controller/CalendarController.php +++ b/lib/Controller/CalendarController.php @@ -476,6 +476,17 @@ class CalendarController extends Controller */ private function parseAttendees(string $input): array { + /* + * Bound the raw input: splitting an arbitrarily large + * string is a cheap CPU sink and the value is echoed + * back into the page on a re-render. + */ + $this->assertBoundedLength( + $input, + self::MAX_ATTENDEES * (self::MAX_EMAIL_LENGTH + 1), + $this->l10n->t('Invitee list is too long.') + ); + $attendees = []; $seen = []; @@ -522,11 +533,28 @@ class CalendarController extends Controller { $contacts = []; - $results = $this->contactsManager->search(trim($query), ['FN', 'EMAIL']); + /* + * The query is the fragment being typed, a full email + * address is the longest useful input. + */ + $query = mb_substr(trim($query), 0, self::MAX_EMAIL_LENGTH); + + $results = $this->contactsManager->search($query, ['FN', 'EMAIL']); foreach ($results ?? [] as $result) { $name = (string)($result['FN'] ?? ''); + /* + * Contact names become the CN parameter of the ICS + * ATTENDEE. Sabre escapes most special characters on + * serialization, but a bare carriage return bypasses + * its quoting, so names with control characters are + * dropped. + */ + if (preg_match('/[\x00-\x1F\x7F]/u', $name) === 1) { + $name = ''; + } + $emails = $result['EMAIL'] ?? []; $emails = is_array($emails) ? $emails : [$emails]; diff --git a/tests/Unit/CalendarControllerTest.php b/tests/Unit/CalendarControllerTest.php index ee84675..b06d657 100644 --- a/tests/Unit/CalendarControllerTest.php +++ b/tests/Unit/CalendarControllerTest.php @@ -933,6 +933,16 @@ class CalendarControllerTest extends TestCase $this->invoke($c, 'parseAttendees', ['alice@example.com,not-an-email']); } + public function testParseAttendeesRejectsOversizedInput(): void + { + $c = $this->newController(); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invitee list is too long.'); + + $this->invoke($c, 'parseAttendees', [str_repeat('x', 6000)]); + } + public function testParseAttendeesEnforcesLimit(): void { $c = $this->newController(); @@ -1045,4 +1055,42 @@ class CalendarControllerTest extends TestCase $this->assertCount(50, $response->getData()); } + + public function testSearchContactsTruncatesQuery(): void + { + $this->contactsManager->expects($this->once())->method('search') + ->willReturnCallback( + static function (string $query): array { + return strlen($query) <= 254 + ? [['FN' => 'Alice', 'EMAIL' => 'alice@example.com']] + : []; + } + ); + + $c = $this->newController(); + + $response = $c->searchContacts(str_repeat('a', 300)); + + $this->assertSame( + [['email' => 'alice@example.com', 'name' => 'Alice']], + $response->getData() + ); + } + + public function testContactSuggestionsDropControlCharNames(): void + { + $this->contactsManager->method('search')->willReturn([ + ['FN' => "Evil\rName", 'EMAIL' => 'evil@example.com'], + ['FN' => 'Alice', 'EMAIL' => 'alice@example.com'], + ]); + + $c = $this->newController(); + + $suggestions = $this->invoke($c, 'contactSuggestions', ['']); + + $this->assertSame([ + ['email' => 'evil@example.com', 'name' => ''], + ['email' => 'alice@example.com', 'name' => 'Alice'], + ], $suggestions); + } }