From ea117bac315b10ad50cb6bdcc56799842d59ad25 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Sun, 22 Oct 2017 12:16:58 +0200 Subject: [PATCH] catch errors when parsing calendar data for calendar query requests Signed-off-by: Georg Ehrke --- apps/dav/appinfo/v1/caldav.php | 3 +- apps/dav/lib/CalDAV/CalDavBackend.php | 29 ++++++++++++++++++- apps/dav/lib/Command/CreateCalendar.php | 3 +- apps/dav/lib/RootCollection.php | 3 +- .../unit/CalDAV/AbstractCalDavBackend.php | 6 +++- .../unit/CalDAV/PublicCalendarRootTest.php | 5 ++++ 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index e96c3f2806..d9851bf92c 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -49,8 +49,9 @@ $principalBackend = new Principal( $db = \OC::$server->getDatabaseConnection(); $userManager = \OC::$server->getUserManager(); $random = \OC::$server->getSecureRandom(); +$logger = \OC::$server->getLogger(); $dispatcher = \OC::$server->getEventDispatcher(); -$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $dispatcher, true); +$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $logger, $dispatcher, true); $debugging = \OC::$server->getConfig()->getSystemValue('debug', false); $sendInvitations = \OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes'; diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 9045a62cde..2f591a262a 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -37,6 +37,7 @@ use OCA\DAV\Connector\Sabre\Principal; use OCA\DAV\DAV\Sharing\Backend; use OCP\IDBConnection; use OCP\IGroupManager; +use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; use OCP\Security\ISecureRandom; @@ -56,6 +57,8 @@ use Sabre\VObject\Component\VCalendar; use Sabre\VObject\Component\VEvent; use Sabre\VObject\Component\VTimeZone; use Sabre\VObject\DateTimeParser; +use Sabre\VObject\InvalidDataException; +use Sabre\VObject\ParseException; use Sabre\VObject\Property; use Sabre\VObject\Reader; use Sabre\VObject\Recur\EventIterator; @@ -152,6 +155,9 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription /** @var ISecureRandom */ private $random; + /** @var ILogger */ + private $logger; + /** @var EventDispatcherInterface */ private $dispatcher; @@ -169,6 +175,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription * @param IUserManager $userManager * @param IGroupManager $groupManager * @param ISecureRandom $random + * @param ILogger $logger * @param EventDispatcherInterface $dispatcher * @param bool $legacyEndpoint */ @@ -177,6 +184,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription IUserManager $userManager, IGroupManager $groupManager, ISecureRandom $random, + ILogger $logger, EventDispatcherInterface $dispatcher, $legacyEndpoint = false) { $this->db = $db; @@ -184,6 +192,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription $this->userManager = $userManager; $this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'calendar'); $this->random = $random; + $this->logger = $logger; $this->dispatcher = $dispatcher; $this->legacyEndpoint = $legacyEndpoint; } @@ -1219,7 +1228,25 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription $result = []; while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { if ($requirePostFilter) { - if (!$this->validateFilterForObject($row, $filters)) { + // validateFilterForObject will parse the calendar data + // catch parsing errors + try { + $matches = $this->validateFilterForObject($row, $filters); + } catch(ParseException $ex) { + $this->logger->logException($ex, [ + 'app' => 'dav', + 'message' => 'Caught parsing exception for calendar data. This usually indicates invalid calendar data. calendar-id:'.$calendarId.' uri:'.$row['uri'] + ]); + continue; + } catch (InvalidDataException $ex) { + $this->logger->logException($ex, [ + 'app' => 'dav', + 'message' => 'Caught invalid data exception for calendar data. This usually indicates invalid calendar data. calendar-id:'.$calendarId.' uri:'.$row['uri'] + ]); + continue; + } + + if (!$matches) { continue; } } diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php index 1ef859e063..190e4aa339 100644 --- a/apps/dav/lib/Command/CreateCalendar.php +++ b/apps/dav/lib/Command/CreateCalendar.php @@ -78,10 +78,11 @@ class CreateCalendar extends Command { $this->groupManager ); $random = \OC::$server->getSecureRandom(); + $logger = \OC::$server->getLogger(); $dispatcher = \OC::$server->getEventDispatcher(); $name = $input->getArgument('name'); - $caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $dispatcher); + $caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $logger, $dispatcher); $caldav->createCalendar("principals/users/$user", $name, []); } } diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index e4ba1f2c02..7af1745cd7 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -40,6 +40,7 @@ class RootCollection extends SimpleCollection { public function __construct() { $config = \OC::$server->getConfig(); $random = \OC::$server->getSecureRandom(); + $logger = \OC::$server->getLogger(); $userManager = \OC::$server->getUserManager(); $groupManager = \OC::$server->getGroupManager(); $db = \OC::$server->getDatabaseConnection(); @@ -61,7 +62,7 @@ class RootCollection extends SimpleCollection { $systemPrincipals->disableListing = $disableListing; $filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users'); $filesCollection->disableListing = $disableListing; - $caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $dispatcher); + $caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $logger, $dispatcher); $calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users'); $calendarRoot->disableListing = $disableListing; $publicCalendarRoot = new PublicCalendarRoot($caldavBackend); diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index 90bf860b24..2a01bd425c 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -28,6 +28,7 @@ namespace OCA\DAV\Tests\unit\CalDAV; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\Connector\Sabre\Principal; use OCP\IGroupManager; +use OCP\ILogger; use OCP\IUserManager; use OCP\Security\ISecureRandom; use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; @@ -57,6 +58,8 @@ abstract class AbstractCalDavBackend extends TestCase { /** @var ISecureRandom */ private $random; + /** @var ILogger */ + private $logger; const UNIT_TEST_USER = 'principals/users/caldav-unit-test'; const UNIT_TEST_USER1 = 'principals/users/caldav-unit-test1'; @@ -84,7 +87,8 @@ abstract class AbstractCalDavBackend extends TestCase { $db = \OC::$server->getDatabaseConnection(); $this->random = \OC::$server->getSecureRandom(); - $this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->dispatcher); + $this->logger = $this->createMock(ILogger::class); + $this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->logger, $this->dispatcher); $this->cleanUpBackend(); } diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php index 82f4161c20..57707c6c0a 100644 --- a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php @@ -34,6 +34,7 @@ use OCP\IGroupManager; use OCP\IL10N; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\PublicCalendarRoot; +use OCP\ILogger; use OCP\IUserManager; use OCP\Security\ISecureRandom; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -64,6 +65,8 @@ class PublicCalendarRootTest extends TestCase { /** @var ISecureRandom */ private $random; + /** @var ILogger */ + private $logger; public function setUp() { parent::setUp(); @@ -73,6 +76,7 @@ class PublicCalendarRootTest extends TestCase { $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->random = \OC::$server->getSecureRandom(); + $this->logger = $this->createMock(ILogger::class); $dispatcher = $this->createMock(EventDispatcherInterface::class); $this->principal->expects($this->any())->method('getGroupMembership') @@ -85,6 +89,7 @@ class PublicCalendarRootTest extends TestCase { $this->userManager, $this->groupManager, $this->random, + $this->logger, $dispatcher );