Skip to content

Commit 66e5d46

Browse files
Merge pull request #55859 from nextcloud/backport/55856/stable25
[stable25] fix(dav): Restrict properties allowed object classes
2 parents f1358bf + 4db1243 commit 66e5d46

File tree

1 file changed

+40
-4
lines changed

1 file changed

+40
-4
lines changed

apps/dav/lib/DAV/CustomPropertiesBackend.php

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use OCP\DB\QueryBuilder\IQueryBuilder;
3131
use OCP\IDBConnection;
3232
use OCP\IUser;
33+
use Sabre\DAV\Exception as DavException;
3334
use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
3435
use Sabre\DAV\PropFind;
3536
use Sabre\DAV\PropPatch;
@@ -373,7 +374,7 @@ private function updateProperties(string $path, array $properties): bool {
373374
->executeStatement();
374375
}
375376
} else {
376-
[$value, $valueType] = $this->encodeValueForDatabase($propertyValue);
377+
[$value, $valueType] = $this->encodeValueForDatabase($propertyName, $propertyValue);
377378
$dbParameters['propertyValue'] = $value;
378379
$dbParameters['valueType'] = $valueType;
379380

@@ -415,17 +416,46 @@ private function formatPath(string $path): string {
415416
return $path;
416417
}
417418

419+
private static function checkIsArrayOfScalar(string $name, array $array): void {
420+
foreach ($array as $item) {
421+
if (is_array($item)) {
422+
self::checkIsArrayOfScalar($name, $item);
423+
} elseif ($item !== null && !is_scalar($item)) {
424+
throw new DavException(
425+
"Property \"$name\" has an invalid value of array containing " . gettype($item),
426+
);
427+
}
428+
}
429+
}
430+
418431
/**
419432
* @param mixed $value
420433
* @return array
421434
*/
422-
private function encodeValueForDatabase($value): array {
435+
private function encodeValueForDatabase(string $name, $value): array {
423436
if (is_scalar($value)) {
424437
$valueType = self::PROPERTY_TYPE_STRING;
425438
} elseif ($value instanceof Complex) {
426439
$valueType = self::PROPERTY_TYPE_XML;
427440
$value = $value->getXml();
428441
} else {
442+
if (is_array($value)) {
443+
// For array only allow scalar values
444+
self::checkIsArrayOfScalar($name, $value);
445+
} elseif (!is_object($value)) {
446+
throw new DavException(
447+
"Property \"$name\" has an invalid value of type " . gettype($value),
448+
);
449+
} else {
450+
if (!str_starts_with(get_class($value), 'Sabre\\DAV\\Xml\\Property\\')
451+
&& !str_starts_with(get_class($value), 'Sabre\\CalDAV\\Xml\\Property\\')
452+
&& !str_starts_with(get_class($value), 'Sabre\\CardDAV\\Xml\\Property\\')
453+
&& !str_starts_with(get_class($value), 'OCA\\DAV\\')) {
454+
throw new DavException(
455+
"Property \"$name\" has an invalid value of class " . get_class($value),
456+
);
457+
}
458+
}
429459
$valueType = self::PROPERTY_TYPE_OBJECT;
430460
$value = serialize($value);
431461
}
@@ -440,11 +470,17 @@ private function decodeValueFromDatabase(string $value, int $valueType) {
440470
case self::PROPERTY_TYPE_XML:
441471
return new Complex($value);
442472
case self::PROPERTY_TYPE_OBJECT:
473+
if (preg_match('/^a:/', $value)) {
474+
// Array, unserialize only scalar values
475+
return unserialize(str_replace('\x00', chr(0), $value), ['allowed_classes' => false]);
476+
}
477+
if (!preg_match('/^O\:\d+\:\"(OCA\\\\DAV\\\\|Sabre\\\\(Cal|Card)?DAV\\\\Xml\\\\Property\\\\)/', $value)) {
478+
throw new \LogicException('Found an object class serialized in DB that is not allowed');
479+
}
443480
return unserialize($value);
444-
case self::PROPERTY_TYPE_STRING:
445481
default:
446482
return $value;
447-
}
483+
};
448484
}
449485

450486
private function createDeleteQuery(): IQueryBuilder {

0 commit comments

Comments
 (0)