Skip to content

Commit f59155d

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 e511ce5 commit f59155d

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
@@ -76,38 +76,53 @@ public static function readPropertyWithDefault(VEvent $vevent, string $property,
7676
return $default;
7777
}
7878

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

93106
/**
94107
* Like generateDiffString() but linkifies the property values if they are urls.
95108
*/
96-
private function generateLinkifiedDiffString(VEvent $vevent, VEvent $oldVEvent, string $property, string $default): ?string {
97-
if (!isset($vevent->$property)) {
98-
return $default;
109+
private function generateLinkifiedDiffString(VEvent $vEvent, VEvent $oldVEvent, string $property): ?string {
110+
if (!isset($vEvent->$property)) {
111+
return null;
99112
}
100-
$value = $vevent->$property->getValue();
101-
$newString = $value === null ? null : htmlspecialchars($value);
102-
$oldString = isset($oldVEvent->$property) ? htmlspecialchars($oldVEvent->$property->getValue()) : null;
103-
if ($oldString !== $newString) {
104-
return sprintf(
105-
"<span style='text-decoration: line-through'>%s</span><br />%s",
106-
$this->linkify($oldString) ?? $oldString ?? '',
107-
$this->linkify($newString) ?? $newString ?? ''
108-
);
113+
114+
$newValue = $vEvent->$property->getValue();
115+
$newString = $this->linkify($newValue) ?? htmlspecialchars($newValue);
116+
117+
$propertyChanged = isset($oldVEvent->$property) && $oldVEvent->$property->getValue() !== $newValue;
118+
if ($propertyChanged) {
119+
$oldValue = $oldVEvent->$property->getValue();
120+
$oldString = $this->linkify($oldValue) ?? htmlspecialchars($oldValue);
121+
122+
return $this->getStrikethroughString($oldString, $newString);
109123
}
110-
return $this->linkify($newString) ?? $newString;
124+
125+
return $this->getStrikethroughString($newString);
111126
}
112127

113128
/**
@@ -117,7 +132,15 @@ private function linkify(?string $url): ?string {
117132
if ($url === null) {
118133
return null;
119134
}
120-
if (!str_starts_with($url, 'http://') && !str_starts_with($url, 'https://')) {
135+
136+
$isValidLinkUrl
137+
= filter_var($url, FILTER_VALIDATE_URL) !== false
138+
&& (
139+
str_starts_with($url, 'http://')
140+
|| str_starts_with($url, 'https://')
141+
);
142+
143+
if (!$isValidLinkUrl) {
121144
return null;
122145
}
123146

@@ -130,7 +153,6 @@ private function linkify(?string $url): ?string {
130153
* @return array
131154
*/
132155
public function buildBodyData(VEvent $vEvent, ?VEvent $oldVEvent): array {
133-
134156
// construct event reader
135157
$eventReaderCurrent = new EventReader($vEvent);
136158
$eventReaderPrevious = !empty($oldVEvent) ? new EventReader($oldVEvent) : null;
@@ -142,22 +164,26 @@ public function buildBodyData(VEvent $vEvent, ?VEvent $oldVEvent): array {
142164
$data[$key] = self::readPropertyWithDefault($vEvent, $property, $defaultVal);
143165
}
144166

145-
$data['meeting_url_html'] = self::readPropertyWithDefault($vEvent, 'URL', $defaultVal);
146-
147-
if (($locationHtml = $this->linkify($data['meeting_location'])) !== null) {
148-
$data['meeting_location_html'] = $locationHtml;
149-
}
167+
$data['meeting_location_html'] = $this->linkify($data['meeting_location']);
168+
$data['meeting_url_html'] = $this->linkify($data['meeting_url']);
150169

151170
if (!empty($oldVEvent)) {
152-
$oldMeetingWhen = $this->generateWhenString($eventReaderPrevious);
153-
$data['meeting_title_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'SUMMARY', $data['meeting_title']);
154-
$data['meeting_description_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'DESCRIPTION', $data['meeting_description']);
155-
$data['meeting_location_html'] = $this->generateLinkifiedDiffString($vEvent, $oldVEvent, 'LOCATION', $data['meeting_location']);
156-
157-
$oldUrl = self::readPropertyWithDefault($oldVEvent, 'URL', $defaultVal);
158-
$data['meeting_url_html'] = !empty($oldUrl) && $oldUrl !== $data['meeting_url'] ? sprintf('<a href="%1$s">%1$s</a>', $oldUrl) : $data['meeting_url'];
171+
$data['meeting_title_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'SUMMARY');
172+
$data['meeting_description_html'] = $this->generateDiffString($vEvent, $oldVEvent, 'DESCRIPTION');
173+
$data['meeting_location_html'] = $this->generateLinkifiedDiffString($vEvent, $oldVEvent, 'LOCATION');
174+
175+
$oldMeetingUrl = self::readPropertyWithDefault($oldVEvent, 'URL', $defaultVal);
176+
$oldMeetingUrlAsLink = $this->linkify($oldMeetingUrl);
177+
$meetingUrlAsLinkChanged = !empty($oldMeetingUrlAsLink) && $oldMeetingUrlAsLink !== $data['meeting_url_html'];
178+
if ($meetingUrlAsLinkChanged) {
179+
$data['meeting_url_html'] = $this->getStrikethroughString(htmlspecialchars($oldMeetingUrl), $data['meeting_url_html']);
180+
}
159181

160-
$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'];
182+
$oldMeetingWhen = $this->generateWhenString($eventReaderPrevious);
183+
$meetingWhenChanged = $oldMeetingWhen !== $data['meeting_when'];
184+
$data['meeting_when_html'] = $meetingWhenChanged
185+
? $this->getStrikethroughString($oldMeetingWhen, $data['meeting_when'])
186+
: null;
161187
}
162188
// generate occurring next string
163189
if ($eventReaderCurrent->recurs()) {
@@ -181,11 +207,8 @@ public function buildReplyBodyData(VEvent $vEvent): array {
181207
$data[$key] = self::readPropertyWithDefault($vEvent, $property, $defaultVal);
182208
}
183209

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

190213
// generate occurring next string
191214
if ($eventReader->recurs()) {
@@ -468,7 +491,7 @@ public function generateWhenStringRecurringMonthly(EventReader $er): string {
468491
// days of month
469492
if ($er->recurringPattern() === 'R') {
470493
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
471-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
494+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
472495
} else {
473496
$days = implode(', ', $er->recurringDaysOfMonth());
474497
}
@@ -535,7 +558,7 @@ public function generateWhenStringRecurringYearly(EventReader $er): string {
535558
// days of month
536559
if ($er->recurringPattern() === 'R') {
537560
$days = implode(', ', array_map(function ($value) { return $this->localizeRelativePositionName($value); }, $er->recurringRelativePositionNamed())) . ' '
538-
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
561+
. implode(', ', array_map(function ($value) { return $this->localizeDayName($value); }, $er->recurringDaysOfWeekNamed()));
539562
} else {
540563
$days = $er->startDateTime()->format('jS');
541564
}
@@ -625,7 +648,6 @@ public function generateWhenStringRecurringFixed(EventReader $er): string {
625648
* @return string
626649
*/
627650
public function generateOccurringString(EventReader $er): string {
628-
629651
// initialize
630652
$occurrence = null;
631653
$occurrence2 = null;
@@ -796,26 +818,26 @@ public function buildCancelledBodyData(VEvent $vEvent): array {
796818
// construct event reader
797819
$eventReaderCurrent = new EventReader($vEvent);
798820
$defaultVal = '';
799-
$strikethrough = "<span style='text-decoration: line-through'>%s</span>";
800821

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

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

0 commit comments

Comments
 (0)