Skip to content

Commit acce1c2

Browse files
DerDreschnerDerDreschner
committed
fix(ImipService): Make sure non-html fields are escaped and html fields are not
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
1 parent c2e72d3 commit acce1c2

2 files changed

Lines changed: 426 additions & 63 deletions

File tree

‎apps/dav/lib/CalDAV/Schedule/IMipService.php‎

Lines changed: 78 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -78,38 +78,53 @@ public static function readPropertyWithDefault(VEvent $vevent, string $property,
7878
return $default;
7979
}
8080

81-
private function generateDiffString(VEvent $vevent, VEvent $oldVEvent, string $property, string $default): ?string {
82-
$strikethrough = "<span style='text-decoration: line-through'>%s</span><br />%s";
83-
if (!isset($vevent->$property)) {
84-
return $default;
81+
private function getStrikethroughString(?string $oldString, ?string $newValue = null): ?string {
82+
if ($oldString === null || $oldString === '') {
83+
return null;
8584
}
86-
$value = $vevent->$property->getValue();
87-
$newstring = $value === null ? null : htmlspecialchars($value);
88-
if (isset($oldVEvent->$property) && $oldVEvent->$property->getValue() !== $newstring) {
89-
$oldstring = $oldVEvent->$property->getValue();
90-
return sprintf($strikethrough, htmlspecialchars($oldstring), $newstring);
85+
86+
$strikethrough = '<span style="text-decoration: line-through">%s</span><br />%s';
87+
return sprintf($strikethrough, $oldString, $newValue ?? '');
88+
}
89+
90+
private function generateDiffString(VEvent $vEvent, VEvent $oldVEvent, string $property): ?string {
91+
if (!isset($vEvent->$property)) {
92+
return null;
9193
}
92-
return $newstring;
94+
95+
$newValue = $vEvent->$property->getValue();
96+
$newString = $newValue === null ? null : htmlspecialchars($newValue);
97+
98+
$propertyChanged = isset($oldVEvent->$property) && $oldVEvent->$property->getValue() !== $newString;
99+
if ($propertyChanged) {
100+
$oldValue = $oldVEvent->$property->getValue();
101+
$oldString = htmlspecialchars($oldValue);
102+
103+
return $this->getStrikethroughString($oldString, $newString);
104+
}
105+
return $newString;
93106
}
94107

95108
/**
96109
* Like generateDiffString() but linkifies the property values if they are urls.
97110
*/
98-
private function generateLinkifiedDiffString(VEvent $vevent, VEvent $oldVEvent, string $property, string $default): ?string {
99-
if (!isset($vevent->$property)) {
100-
return $default;
111+
private function generateLinkifiedDiffString(VEvent $vEvent, VEvent $oldVEvent, string $property): ?string {
112+
if (!isset($vEvent->$property)) {
113+
return null;
101114
}
102-
/** @var string|null $newString */
103-
$newString = htmlspecialchars($vevent->$property->getValue());
104-
$oldString = isset($oldVEvent->$property) ? htmlspecialchars($oldVEvent->$property->getValue()) : null;
105-
if ($oldString !== $newString) {
106-
return sprintf(
107-
"<span style='text-decoration: line-through'>%s</span><br />%s",
108-
$this->linkify($oldString) ?? $oldString ?? '',
109-
$this->linkify($newString) ?? $newString ?? ''
110-
);
115+
116+
$newValue = $vEvent->$property->getValue();
117+
$newString = $this->linkify($newValue) ?? htmlspecialchars($newValue);
118+
119+
$propertyChanged = isset($oldVEvent->$property) && $oldVEvent->$property->getValue() !== $newValue;
120+
if ($propertyChanged) {
121+
$oldValue = $oldVEvent->$property->getValue();
122+
$oldString = $this->linkify($oldValue) ?? htmlspecialchars($oldValue);
123+
124+
return $this->getStrikethroughString($oldString, $newString);
111125
}
112-
return $this->linkify($newString) ?? $newString;
126+
127+
return $this->getStrikethroughString($newString);
113128
}
114129

115130
/**
@@ -119,7 +134,15 @@ private function linkify(?string $url): ?string {
119134
if ($url === null) {
120135
return null;
121136
}
122-
if (!str_starts_with($url, 'http://') && !str_starts_with($url, 'https://')) {
137+
138+
$isValidLinkUrl
139+
= filter_var($url, FILTER_VALIDATE_URL) !== false
140+
&& (
141+
str_starts_with($url, 'http://')
142+
|| str_starts_with($url, 'https://')
143+
);
144+
145+
if (!$isValidLinkUrl) {
123146
return null;
124147
}
125148

@@ -132,7 +155,6 @@ private function linkify(?string $url): ?string {
132155
* @return array
133156
*/
134157
public function buildBodyData(VEvent $vEvent, ?VEvent $oldVEvent): array {
135-
136158
// construct event reader
137159
$eventReaderCurrent = new EventReader($vEvent);
138160
$eventReaderPrevious = !empty($oldVEvent) ? new EventReader($oldVEvent) : null;
@@ -144,22 +166,25 @@ public function buildBodyData(VEvent $vEvent, ?VEvent $oldVEvent): array {
144166
$data[$key] = self::readPropertyWithDefault($vEvent, $property, $defaultVal);
145167
}
146168

147-
$data['meeting_url_html'] = self::readPropertyWithDefault($vEvent, 'URL', $defaultVal);
148-
149-
if (($locationHtml = $this->linkify($data['meeting_location'])) !== null) {
150-
$data['meeting_location_html'] = $locationHtml;
151-
}
169+
$data['meeting_location_html'] = $this->linkify($data['meeting_location']);
170+
$data['meeting_url_html'] = $this->linkify($data['meeting_url']);
152171

153172
if (!empty($oldVEvent)) {
154173
$oldMeetingWhen = $this->generateWhenString($eventReaderPrevious);
155-
$data['meeting_title_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'SUMMARY', $data['meeting_title']);
156-
$data['meeting_description_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'DESCRIPTION', $data['meeting_description']);
157-
$data['meeting_location_html'] = $this->generateLinkifiedDiffString($vEvent, $oldVEvent, 'LOCATION', $data['meeting_location']);
158-
159-
$oldUrl = self::readPropertyWithDefault($oldVEvent, 'URL', $defaultVal);
160-
$data['meeting_url_html'] = !empty($oldUrl) && $oldUrl !== $data['meeting_url'] ? sprintf('<a href="%1$s">%1$s</a>', $oldUrl) : $data['meeting_url'];
174+
$data['meeting_title_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'SUMMARY');
175+
$data['meeting_description_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'DESCRIPTION');
176+
$data['meeting_location_html'] = $this->generateLinkifiedDiffString($vEvent, $oldVEvent, 'LOCATION');
177+
178+
$meetingUrlChanged = !empty($oldMeetingUrl) && $oldMeetingUrl !== $data['meeting_url'];
179+
if ($meetingUrlChanged) {
180+
$oldMeetingUrl = self::readPropertyWithDefault($oldVEvent, 'URL', $defaultVal);
181+
$data['meeting_url_html'] = $this->linkify($oldMeetingUrl);
182+
}
161183

162-
$data['meeting_when_html'] = $oldMeetingWhen !== $data['meeting_when'] ? sprintf("<span style='text-decoration: line-through'>%s</span><br />%s", $oldMeetingWhen, $data['meeting_when']) : $data['meeting_when'];
184+
$meetingWhenChanged = $oldMeetingWhen !== $data['meeting_when'];
185+
$data['meeting_when_html'] = $meetingWhenChanged
186+
? $this->getStrikethroughString($oldMeetingWhen, $data['meeting_when'])
187+
: null;
163188
}
164189
// generate occurring next string
165190
if ($eventReaderCurrent->recurs()) {
@@ -183,11 +208,8 @@ public function buildReplyBodyData(VEvent $vEvent): array {
183208
$data[$key] = self::readPropertyWithDefault($vEvent, $property, $defaultVal);
184209
}
185210

186-
if (($locationHtml = $this->linkify($data['meeting_location'])) !== null) {
187-
$data['meeting_location_html'] = $locationHtml;
188-
}
189-
190-
$data['meeting_url_html'] = $data['meeting_url'] ? sprintf('<a href="%1$s">%1$s</a>', $data['meeting_url']) : '';
211+
$data['meeting_location_html'] = $this->linkify($data['meeting_location']);
212+
$data['meeting_url_html'] = $this->linkify($data['meeting_url']);
191213

192214
// generate occurring next string
193215
if ($eventReader->recurs()) {
@@ -470,7 +492,7 @@ public function generateWhenStringRecurringMonthly(EventReader $er): string {
470492
// days of month
471493
if ($er->recurringPattern() === 'R') {
472494
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
473-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
495+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
474496
} else {
475497
$days = implode(', ', $er->recurringDaysOfMonth());
476498
}
@@ -537,7 +559,7 @@ public function generateWhenStringRecurringYearly(EventReader $er): string {
537559
// days of month
538560
if ($er->recurringPattern() === 'R') {
539561
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
540-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
562+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
541563
} else {
542564
$days = $er->startDateTime()->format('jS');
543565
}
@@ -627,7 +649,6 @@ public function generateWhenStringRecurringFixed(EventReader $er): string {
627649
* @return string
628650
*/
629651
public function generateOccurringString(EventReader $er): string {
630-
631652
// initialize
632653
$occurrence = null;
633654
$occurrence2 = null;
@@ -798,26 +819,26 @@ public function buildCancelledBodyData(VEvent $vEvent): array {
798819
// construct event reader
799820
$eventReaderCurrent = new EventReader($vEvent);
800821
$defaultVal = '';
801-
$strikethrough = "<span style='text-decoration: line-through'>%s</span>";
802822

803823
$newMeetingWhen = $this->generateWhenString($eventReaderCurrent);
804-
$newSummary = htmlspecialchars(isset($vEvent->SUMMARY) && (string)$vEvent->SUMMARY !== '' ? (string)$vEvent->SUMMARY : $this->l10n->t('Untitled event'));
805-
$newDescription = htmlspecialchars(isset($vEvent->DESCRIPTION) && (string)$vEvent->DESCRIPTION !== '' ? (string)$vEvent->DESCRIPTION : $defaultVal);
806-
$newUrl = isset($vEvent->URL) && (string)$vEvent->URL !== '' ? sprintf('<a href="%1$s">%1$s</a>', $vEvent->URL) : $defaultVal;
807-
$newLocation = htmlspecialchars(isset($vEvent->LOCATION) && (string)$vEvent->LOCATION !== '' ? (string)$vEvent->LOCATION : $defaultVal);
808-
$newLocationHtml = $this->linkify($newLocation) ?? $newLocation;
824+
$newSummary = isset($vEvent->SUMMARY) && (string)$vEvent->SUMMARY !== '' ? (string)$vEvent->SUMMARY : $this->l10n->t('Untitled event');
825+
$newDescription = isset($vEvent->DESCRIPTION) && (string)$vEvent->DESCRIPTION !== '' ? (string)$vEvent->DESCRIPTION : $defaultVal;
826+
$newUrl = isset($vEvent->URL) && (string)$vEvent->URL !== '' ? $this->linkify((string)$vEvent->URL) : $defaultVal;
827+
$newLocation = isset($vEvent->LOCATION) && (string)$vEvent->LOCATION !== '' ? (string)$vEvent->LOCATION : $defaultVal;
828+
$newLocationHtml = $this->linkify($newLocation);
809829

810830
$data = [];
811-
$data['meeting_when_html'] = $newMeetingWhen === '' ?: sprintf($strikethrough, $newMeetingWhen);
831+
$data['meeting_when_html'] = $this->getStrikethroughString(htmlspecialchars($newMeetingWhen));
812832
$data['meeting_when'] = $newMeetingWhen;
813-
$data['meeting_title_html'] = sprintf($strikethrough, $newSummary);
833+
$data['meeting_title_html'] = $this->getStrikethroughString(htmlspecialchars($newSummary));
814834
$data['meeting_title'] = $newSummary !== '' ? $newSummary: $this->l10n->t('Untitled event');
815-
$data['meeting_description_html'] = $newDescription !== '' ? sprintf($strikethrough, $newDescription) : '';
835+
$data['meeting_description_html'] = $this->getStrikethroughString(htmlspecialchars($newDescription));
816836
$data['meeting_description'] = $newDescription;
817-
$data['meeting_url_html'] = $newUrl !== '' ? sprintf($strikethrough, $newUrl) : '';
837+
$data['meeting_url_html'] = $this->getStrikethroughString($newUrl);
818838
$data['meeting_url'] = isset($vEvent->URL) ? (string)$vEvent->URL : '';
819-
$data['meeting_location_html'] = $newLocationHtml !== '' ? sprintf($strikethrough, $newLocationHtml) : '';
839+
$data['meeting_location_html'] = $this->getStrikethroughString($newLocationHtml ?? htmlspecialchars($newLocation));
820840
$data['meeting_location'] = $newLocation;
841+
821842
return $data;
822843
}
823844

@@ -953,7 +974,7 @@ public function getAttendeeRsvpOrReqForParticipant(?Property $attendee = null) {
953974
* @param bool $isModified
954975
*/
955976
public function addSubjectAndHeading(IEMailTemplate $template,
956-
string $method, string $sender, string $summary, bool $isModified, ?Property $replyingAttendee = null): void {
977+
string $method, string $sender, string $summary, bool $isModified, ?Property $replyingAttendee = null): void {
957978
if ($method === IMipPlugin::METHOD_CANCEL) {
958979
// TRANSLATORS Subject for email, when an invitation is cancelled. Ex: "Cancelled: {{Event Name}}"
959980
$template->setSubject($this->l10n->t('Cancelled: %1$s', [$summary]));

0 commit comments

Comments
 (0)