Skip to content

Commit

Permalink
fix(carddav): limit vcard size
Browse files Browse the repository at this point in the history
Signed-off-by: SebastianKrupinski <[email protected]>
  • Loading branch information
SebastianKrupinski committed Jul 4, 2024
1 parent 7538d66 commit d9ab32c
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 0 deletions.
2 changes: 2 additions & 0 deletions apps/dav/appinfo/v1/carddav.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use OCA\DAV\CardDAV\AddressBookRoot;
use OCA\DAV\CardDAV\CardDavBackend;
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin;
use OCA\DAV\Connector\LegacyDAVACL;
use OCA\DAV\Connector\Sabre\Auth;
use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin;
Expand Down Expand Up @@ -105,6 +106,7 @@
)));
$server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->get(LoggerInterface::class)));
$server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
$server->addPlugin(\OCP\Server::get(CardDavValidatePlugin::class));

// And off we go!
$server->exec();
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
'OCA\\DAV\\CardDAV\\SyncService' => $baseDir . '/../lib/CardDAV/SyncService.php',
'OCA\\DAV\\CardDAV\\SystemAddressbook' => $baseDir . '/../lib/CardDAV/SystemAddressbook.php',
'OCA\\DAV\\CardDAV\\UserAddressBooks' => $baseDir . '/../lib/CardDAV/UserAddressBooks.php',
'OCA\\DAV\\CardDAV\\Validation\\CardDavValidatePlugin' => $baseDir . '/../lib/CardDAV/Validation/CardDavValidatePlugin.php',
'OCA\\DAV\\CardDAV\\Xml\\Groups' => $baseDir . '/../lib/CardDAV/Xml/Groups.php',
'OCA\\DAV\\Command\\CreateAddressBook' => $baseDir . '/../lib/Command/CreateAddressBook.php',
'OCA\\DAV\\Command\\CreateCalendar' => $baseDir . '/../lib/Command/CreateCalendar.php',
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\CardDAV\\SyncService' => __DIR__ . '/..' . '/../lib/CardDAV/SyncService.php',
'OCA\\DAV\\CardDAV\\SystemAddressbook' => __DIR__ . '/..' . '/../lib/CardDAV/SystemAddressbook.php',
'OCA\\DAV\\CardDAV\\UserAddressBooks' => __DIR__ . '/..' . '/../lib/CardDAV/UserAddressBooks.php',
'OCA\\DAV\\CardDAV\\Validation\\CardDavValidatePlugin' => __DIR__ . '/..' . '/../lib/CardDAV/Validation/CardDavValidatePlugin.php',
'OCA\\DAV\\CardDAV\\Xml\\Groups' => __DIR__ . '/..' . '/../lib/CardDAV/Xml/Groups.php',
'OCA\\DAV\\Command\\CreateAddressBook' => __DIR__ . '/..' . '/../lib/Command/CreateAddressBook.php',
'OCA\\DAV\\Command\\CreateCalendar' => __DIR__ . '/..' . '/../lib/Command/CreateCalendar.php',
Expand Down
40 changes: 40 additions & 0 deletions apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

/*
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\DAV\CardDAV\Validation;

use OCA\DAV\AppInfo\Application;
use OCP\IAppConfig;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;

class CardDavValidatePlugin extends ServerPlugin {

public function __construct(
private IAppConfig $config
) {
}

public function initialize(Server $server): void {
$server->on('beforeMethod:PUT', [$this, 'beforePut']);
}

public function beforePut(RequestInterface $request, ResponseInterface $response): bool {
// evaluate if card size exceeds defined limit
$cardSizeLimit = (int) $this->config->getValue(Application::APP_ID, 'card_size_limit', '5242880');

Check failure

Code scanning / Psalm

UndefinedInterfaceMethod Error

Method OCP\IAppConfig::getValue does not exist

Check failure on line 32 in apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedInterfaceMethod

apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php:32:41: UndefinedInterfaceMethod: Method OCP\IAppConfig::getValue does not exist (see https://psalm.dev/181)
if ((int) $request->getRawServerValue('CONTENT_LENGTH') > $cardSizeLimit) {
throw new Forbidden("VCard object exceeds $cardSizeLimit bytes");
}
// all tests passed return true
return true;
}

}
2 changes: 2 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use OCA\DAV\CardDAV\MultiGetExportPlugin;
use OCA\DAV\CardDAV\PhotoCache;
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin;
use OCA\DAV\Comments\CommentsPlugin;
use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin;
use OCA\DAV\Connector\Sabre\Auth;
Expand Down Expand Up @@ -211,6 +212,7 @@ public function __construct(IRequest $request, string $baseUri) {
));

$this->server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
$this->server->addPlugin(\OCP\Server::get(CardDavValidatePlugin::class));
}

// system tags plugins
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

declare(strict_types=1);

/*
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\DAV\Tests\unit\CardDAV\Validation;

use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin;
use OCP\IAppConfig;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Exception\Forbidden;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Test\TestCase;

class CardDavValidatePluginTest extends TestCase {

private CardDavValidatePlugin $plugin;
private IAppConfig|MockObject $config;
private RequestInterface|MockObject $request;
private ResponseInterface|MockObject $response;

protected function setUp(): void {
parent::setUp();
// construct mock objects
$this->config = $this->createMock(IAppConfig::class);
$this->request = $this->createMock(RequestInterface::class);
$this->response = $this->createMock(ResponseInterface::class);
$this->plugin = new CardDavValidatePlugin(
$this->config,
);
}

public function testPutSizeLessThenLimit(): void {

// construct method responses
$this->config
->method('getValue')
->with('dav', 'card_size_limit', '5242880')
->willReturn('5242880');
$this->request
->method('getRawServerValue')
->with('CONTENT_LENGTH')
->willReturn('1024');
// test condition
$this->assertTrue(
$this->plugin->beforePut($this->request, $this->response)
);

}

public function testPutSizeMoreThenLimit(): void {

// construct method responses
$this->config
->method('getValue')
->with('dav', 'card_size_limit', '5242880')
->willReturn('5242880');
$this->request
->method('getRawServerValue')
->with('CONTENT_LENGTH')
->willReturn('6242880');
$this->expectException(Forbidden::class);
// test condition
$this->plugin->beforePut($this->request, $this->response);

}

}

0 comments on commit d9ab32c

Please sign in to comment.