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.
This commit is contained in:
1 parent
97cea7e90b
commit
6312c18a91
5 files changed
+83
-4
No files matched your search
+2
-1
@@ -34,7 +34,8 @@
|
|||||||
"Invitees": "Teilnehmer",
|
"Invitees": "Teilnehmer",
|
||||||
"Email addresses, comma-separated": "E-Mail-Adressen, durch Kommas getrennt",
|
"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.",
|
"%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);"
|
"pluralForms": "nplurals=2; plural=(n != 1);"
|
||||||
}
|
}
|
||||||
+2
-1
@@ -34,7 +34,8 @@
|
|||||||
"Invitees": "Teilnehmer",
|
"Invitees": "Teilnehmer",
|
||||||
"Email addresses, comma-separated": "E-Mail-Adressen, durch Kommas getrennt",
|
"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.",
|
"%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);"
|
"pluralForms": "nplurals=2; plural=(n != 1);"
|
||||||
}
|
}
|
||||||
+2
-1
@@ -34,7 +34,8 @@
|
|||||||
"Invitees": "Invitees",
|
"Invitees": "Invitees",
|
||||||
"Email addresses, comma-separated": "Email addresses, comma-separated",
|
"Email addresses, comma-separated": "Email addresses, comma-separated",
|
||||||
"%s is not a valid email address.": "%s is not a valid email address.",
|
"%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);"
|
"pluralForms": "nplurals=2; plural=(n != 1);"
|
||||||
}
|
}
|
||||||
@@ -476,6 +476,17 @@ class CalendarController extends Controller
|
|||||||
*/
|
*/
|
||||||
private function parseAttendees(string $input): array
|
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 = [];
|
$attendees = [];
|
||||||
$seen = [];
|
$seen = [];
|
||||||
|
|
||||||
@@ -522,11 +533,28 @@ class CalendarController extends Controller
|
|||||||
{
|
{
|
||||||
$contacts = [];
|
$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) {
|
foreach ($results ?? [] as $result) {
|
||||||
$name = (string)($result['FN'] ?? '');
|
$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 = $result['EMAIL'] ?? [];
|
||||||
$emails = is_array($emails) ? $emails : [$emails];
|
$emails = is_array($emails) ? $emails : [$emails];
|
||||||
|
|
||||||
|
|||||||
@@ -933,6 +933,16 @@ class CalendarControllerTest extends TestCase
|
|||||||
$this->invoke($c, 'parseAttendees', ['alice@example.com,not-an-email']);
|
$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
|
public function testParseAttendeesEnforcesLimit(): void
|
||||||
{
|
{
|
||||||
$c = $this->newController();
|
$c = $this->newController();
|
||||||
@@ -1045,4 +1055,42 @@ class CalendarControllerTest extends TestCase
|
|||||||
|
|
||||||
$this->assertCount(50, $response->getData());
|
$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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user