Skip to content

Commit 241eeed

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 241eeed

2 files changed

Lines changed: 426 additions & 62 deletions

File tree

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

Lines changed: 79 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;
84+
}
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;
8593
}
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);
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);
91104
}
92-
return $newstring;
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,26 @@ 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)) {
154-
$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'];
173+
$data['meeting_title_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'SUMMARY');
174+
$data['meeting_description_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'DESCRIPTION');
175+
$data['meeting_location_html'] = $this->generateLinkifiedDiffString($vEvent, $oldVEvent, 'LOCATION');
176+
177+
$oldMeetingUrl = self::readPropertyWithDefault($oldVEvent, 'URL', $defaultVal);
178+
$oldMeetingUrlAsLink = $this->linkify($oldMeetingUrl);
179+
$meetingUrlAsLinkChanged = !empty($oldMeetingUrlAsLink) && $oldMeetingUrlAsLink !== $data['meeting_url_html'];
180+
if ($meetingUrlAsLinkChanged) {
181+
$data['meeting_url_html'] = $this->getStrikethroughString(htmlspecialchars($oldMeetingUrl), $data['meeting_url_html']);
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+
$oldMeetingWhen = $this->generateWhenString($eventReaderPrevious);
185+
$meetingWhenChanged = $oldMeetingWhen !== $data['meeting_when'];
186+
$data['meeting_when_html'] = $meetingWhenChanged
187+
? $this->getStrikethroughString($oldMeetingWhen, $data['meeting_when'])
188+
: null;
163189
}
164190
// generate occurring next string
165191
if ($eventReaderCurrent->recurs()) {
@@ -183,11 +209,8 @@ public function buildReplyBodyData(VEvent $vEvent): array {
183209
$data[$key] = self::readPropertyWithDefault($vEvent, $property, $defaultVal);
184210
}
185211

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']) : '';
212+
$data['meeting_location_html'] = $this->linkify($data['meeting_location']);
213+
$data['meeting_url_html'] = $this->linkify($data['meeting_url']);
191214

192215
// generate occurring next string
193216
if ($eventReader->recurs()) {
@@ -470,7 +493,7 @@ public function generateWhenStringRecurringMonthly(EventReader $er): string {
470493
// days of month
471494
if ($er->recurringPattern() === 'R') {
472495
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
473-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
496+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
474497
} else {
475498
$days = implode(', ', $er->recurringDaysOfMonth());
476499
}
@@ -537,7 +560,7 @@ public function generateWhenStringRecurringYearly(EventReader $er): string {
537560
// days of month
538561
if ($er->recurringPattern() === 'R') {
539562
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
540-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
563+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
541564
} else {
542565
$days = $er->startDateTime()->format('jS');
543566
}
@@ -627,7 +650,6 @@ public function generateWhenStringRecurringFixed(EventReader $er): string {
627650
* @return string
628651
*/
629652
public function generateOccurringString(EventReader $er): string {
630-
631653
// initialize
632654
$occurrence = null;
633655
$occurrence2 = null;
@@ -798,26 +820,26 @@ public function buildCancelledBodyData(VEvent $vEvent): array {
798820
// construct event reader
799821
$eventReaderCurrent = new EventReader($vEvent);
800822
$defaultVal = '';
801-
$strikethrough = "<span style='text-decoration: line-through'>%s</span>";
802823

803824
$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;
825+
$newSummary = isset($vEvent->SUMMARY) && (string)$vEvent->SUMMARY !== '' ? (string)$vEvent->SUMMARY : $this->l10n->t('Untitled event');
826+
$newDescription = isset($vEvent->DESCRIPTION) && (string)$vEvent->DESCRIPTION !== '' ? (string)$vEvent->DESCRIPTION : $defaultVal;
827+
$newUrl = isset($vEvent->URL) && (string)$vEvent->URL !== '' ? $this->linkify((string)$vEvent->URL) : $defaultVal;
828+
$newLocation = isset($vEvent->LOCATION) && (string)$vEvent->LOCATION !== '' ? (string)$vEvent->LOCATION : $defaultVal;
829+
$newLocationHtml = $this->linkify($newLocation);
809830

810831
$data = [];
811-
$data['meeting_when_html'] = $newMeetingWhen === '' ?: sprintf($strikethrough, $newMeetingWhen);
832+
$data['meeting_when_html'] = $this->getStrikethroughString(htmlspecialchars($newMeetingWhen));
812833
$data['meeting_when'] = $newMeetingWhen;
813-
$data['meeting_title_html'] = sprintf($strikethrough, $newSummary);
834+
$data['meeting_title_html'] = $this->getStrikethroughString(htmlspecialchars($newSummary));
814835
$data['meeting_title'] = $newSummary !== '' ? $newSummary: $this->l10n->t('Untitled event');
815-
$data['meeting_description_html'] = $newDescription !== '' ? sprintf($strikethrough, $newDescription) : '';
836+
$data['meeting_description_html'] = $this->getStrikethroughString(htmlspecialchars($newDescription));
816837
$data['meeting_description'] = $newDescription;
817-
$data['meeting_url_html'] = $newUrl !== '' ? sprintf($strikethrough, $newUrl) : '';
838+
$data['meeting_url_html'] = $this->getStrikethroughString($newUrl);
818839
$data['meeting_url'] = isset($vEvent->URL) ? (string)$vEvent->URL : '';
819-
$data['meeting_location_html'] = $newLocationHtml !== '' ? sprintf($strikethrough, $newLocationHtml) : '';
840+
$data['meeting_location_html'] = $this->getStrikethroughString($newLocationHtml ?? htmlspecialchars($newLocation));
820841
$data['meeting_location'] = $newLocation;
842+
821843
return $data;
822844
}
823845

0 commit comments

Comments
 (0)