Fix: reject CRLF and unknown tokens in ICS option values - #245
Merged
Conversation
alies-dev
force-pushed
the
fix/ics-option-injection
branch
5 times, most recently
from
August 20, 2026 16:07
01a3af0 to
82fde43
Compare
Values passed through the ICS options array were written into the file verbatim. A CR or an LF in UID, PRODID, URL or RRULE ended the property and started another one, so caller supplied data could inject arbitrary calendar content. Those four are validated in the constructor now, and the enumerated TRANSP, CLASS and X-MICROSOFT-CDO-BUSYSTATUS properties are checked against their token lists. A custom REMINDER.DESCRIPTION also skipped escapeString(), unlike the default reminder text, so a semicolon or a comma in it produced an invalid TEXT value. It goes through the same escaping now.
alies-dev
force-pushed
the
fix/ics-option-injection
branch
from
August 20, 2026 16:12
82fde43 to
a7d9724
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context and Purposes
Every value passed through the ICS options array was concatenated into the file as it was given, with no escaping and no validation. An iCalendar stream is a sequence of content lines separated by CRLF (RFC 5545 section 3.1), so a CR or an LF inside one of those values ends the property and starts another one. An application that routes user controlled data into an option therefore let that data write arbitrary calendar content: forge an
ORGANIZER, close theVEVENTearly, append a second event. Lenient parsers treat a bare LF as a line ending, so a single\nwas enough.Three fixes, matching the three points of the issue.
1.
REMINDER.DESCRIPTIONis escaped. AVALARMDESCRIPTIONis a TEXT value (section 3.6.6, section 3.3.11), and the default reminder text already went throughescapeString(). A custom one skipped it, so a;or a,in it produced an invalid TEXT value even with no injection intent. It now takes the same path as the default.2.
UID,PRODID,URLandRRULEreject CR and LF. These four are not TEXT values, soescapeString()cannot be applied to them: aRECURvalue's semicolons and commas are separators (section 3.8.5.3), andURLis a URI (section 3.8.4.6). There is no escape sequence available, so the only correct answer is to refuse the value. Both CR and LF are rejected, not just the CRLF pair.3.
TRANSP,CLASSandX-MICROSOFT-CDO-BUSYSTATUSare checked against their token lists. These take an enumerated token rather than TEXT:OPAQUE|TRANSPARENT(section 3.8.2.7),PUBLIC|PRIVATE|CONFIDENTIAL(section 3.8.1.3) andFREE|TENTATIVE|BUSY|OOF(MS-OXCICAL). The Psalm types onIcsOptionsdocumented these already, but a static type is a contract, not a runtime guarantee, and values that reach the library from a request or a database never pass through a static analyser.Where the validation runs
In the
Icsconstructor, not ingenerate(). The value is then rejected at the point it enters the library, and the stack trace names the caller that supplied it. Validating ingenerate()would surface the failure wherever the link is rendered, often a view far away from the code that built the options, which is the harder failure to read.Link::ics()constructs the generator inline, so callers using that shortcut see no difference in where the exception comes from.Both checks live in small private methods,
guardAgainstLineBreaks()andguardAgainstUnsupportedTokens(). Two named constructors were added toSpatie\CalendarLinks\Exceptions\InvalidLink, following the existinginvalidGuestEmail()style. The line break message deliberately leaves the offending value out: it contains a line break, and would spread the exception message over several lines of a log just as it spread the property over several lines of the calendar.Backwards compatibility
This is a behavioural change for callers currently passing invalid data. Code that passes a CR or an LF in
UID,PRODID,URLorRRULE, or a token outside the allowed list forTRANSP,CLASSorX-MICROSOFT-CDO-BUSYSTATUS, used to produce a broken or attacker controlled calendar file and now throwsInvalidLink(anInvalidArgumentException). Every value that was valid before still generates the same output: no snapshot changed.A custom
REMINDER.DESCRIPTIONcontaining\,;,,or a line break now appears escaped in the output. That is the corrected value rather than a new one, since the unescaped form was not a valid TEXT value to begin with.The
READMEgained a short paragraph pointing out that the event's own fields (the title argument ofLink::create()andLink::createAllDay(),description()andaddress()) are escaped for the caller, and are where user supplied data belongs.Fixes #236