Skip to content

Commit 2959854

Browse files
committed
feat(config): add maximum.supported.desktop.version
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
1 parent 7b1932b commit 2959854

File tree

3 files changed

+98
-18
lines changed

3 files changed

+98
-18
lines changed

apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,22 @@ public function beforeHandler(RequestInterface $request) {
4747
return;
4848
}
4949

50-
$minimumSupportedDesktopVersion = $this->config->getSystemValue('minimum.supported.desktop.version', '2.3.0');
50+
$minimumSupportedDesktopVersion = $this->config->getSystemValueString('minimum.supported.desktop.version', '2.3.0');
51+
$maximumSupportedDesktopVersion = $this->config->getSystemValueString('maximum.supported.desktop.version', '99.99.99');
52+
53+
// Check if the client is a desktop client
5154
preg_match(IRequest::USER_AGENT_CLIENT_DESKTOP, $userAgent, $versionMatches);
52-
if (isset($versionMatches[1]) &&
53-
version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) {
54-
throw new \Sabre\DAV\Exception\Forbidden('Unsupported client version.');
55+
56+
// If the client is a desktop client and the version is too old, block it
57+
if (isset($versionMatches[1]) && version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) {
58+
$minimumSupportedDesktopVersion = htmlspecialchars($minimumSupportedDesktopVersion);
59+
throw new \Sabre\DAV\Exception\Forbidden("This version of the client is unsupported. Upgrade to version $minimumSupportedDesktopVersion or later.");
60+
}
61+
62+
// If the client is a desktop client and the version is too new, block it
63+
if (isset($versionMatches[1]) && version_compare($versionMatches[1], $maximumSupportedDesktopVersion) === 1) {
64+
$maximumSupportedDesktopVersion = htmlspecialchars($maximumSupportedDesktopVersion);
65+
throw new \Sabre\DAV\Exception\Forbidden("This version of the client is unsupported. Downgrade to version $maximumSupportedDesktopVersion or earlier.");
5566
}
5667
}
5768
}

apps/dav/tests/unit/Connector/Sabre/BlockLegacyClientPluginTest.php

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@
1515
use Sabre\HTTP\RequestInterface;
1616
use Test\TestCase;
1717

18+
enum ERROR_TYPE {
19+
case MIN_ERROR;
20+
case MAX_ERROR;
21+
case NONE;
22+
}
23+
1824
/**
1925
* Class BlockLegacyClientPluginTest
2026
*
@@ -33,19 +39,38 @@ protected function setUp(): void {
3339
$this->blockLegacyClientVersionPlugin = new BlockLegacyClientPlugin($this->config);
3440
}
3541

36-
public function oldDesktopClientProvider(): array {
42+
public static function oldDesktopClientProvider(): array {
3743
return [
38-
['Mozilla/5.0 (Windows) mirall/1.5.0'],
39-
['Mozilla/5.0 (Bogus Text) mirall/1.6.9'],
44+
['Mozilla/5.0 (Windows) mirall/1.5.0', ERROR_TYPE::MIN_ERROR],
45+
['Mozilla/5.0 (Bogus Text) mirall/1.6.9', ERROR_TYPE::MIN_ERROR],
46+
['Mozilla/5.0 (Windows) mirall/2.5.0', ERROR_TYPE::MAX_ERROR],
47+
['Mozilla/5.0 (Bogus Text) mirall/2.0.1', ERROR_TYPE::MAX_ERROR],
48+
['Mozilla/5.0 (Windows) mirall/2.0.0', ERROR_TYPE::NONE],
49+
['Mozilla/5.0 (Bogus Text) mirall/2.0.0', ERROR_TYPE::NONE],
4050
];
4151
}
4252

4353
/**
4454
* @dataProvider oldDesktopClientProvider
4555
*/
46-
public function testBeforeHandlerException(string $userAgent): void {
47-
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
48-
$this->expectExceptionMessage('Unsupported client version.');
56+
public function testBeforeHandlerException(string $userAgent, ERROR_TYPE $errorType): void {
57+
$this->config
58+
->expects($this->exactly(2))
59+
->method('getSystemValueString')
60+
->willReturnCallback(function (string $key) {
61+
if ($key === 'minimum.supported.desktop.version') {
62+
return '1.7.0';
63+
}
64+
return '2.0.0';
65+
});
66+
67+
if ($errorType !== ERROR_TYPE::NONE) {
68+
$errorString = $errorType === ERROR_TYPE::MIN_ERROR
69+
? 'This version of the client is unsupported. Upgrade to version 1.7.0 or later.'
70+
: 'This version of the client is unsupported. Downgrade to version 2.0.0 or earlier.';
71+
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
72+
$this->expectExceptionMessage($errorString);
73+
}
4974

5075
/** @var RequestInterface|MockObject $request */
5176
$request = $this->createMock('\Sabre\HTTP\RequestInterface');
@@ -55,20 +80,50 @@ public function testBeforeHandlerException(string $userAgent): void {
5580
->with('User-Agent')
5681
->willReturn($userAgent);
5782

83+
$this->blockLegacyClientVersionPlugin->beforeHandler($request);
84+
}
85+
86+
/**
87+
* Ensure that there is no room for XSS attack through configured URL / version
88+
* @dataProvider oldDesktopClientProvider
89+
*/
90+
public function testBeforeHandlerExceptionPreventXSSAttack(string $userAgent, ERROR_TYPE $errorType): void {
91+
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
92+
5893
$this->config
94+
->expects($this->exactly(2))
95+
->method('getSystemValueString')
96+
->willReturnCallback(function (string $key) {
97+
if ($key === 'minimum.supported.desktop.version') {
98+
return '1.7.0 <script>alert("unsafe")</script>';
99+
}
100+
return '2.0.0 <script>alert("unsafe")</script>';
101+
});
102+
103+
$errorString = $errorType === ERROR_TYPE::MIN_ERROR
104+
? 'This version of the client is unsupported. Upgrade to version 1.7.0 &lt;script&gt;alert(&quot;unsafe&quot;)&lt;/script&gt; or later.'
105+
: 'This version of the client is unsupported. Downgrade to version 2.0.0 &lt;script&gt;alert(&quot;unsafe&quot;)&lt;/script&gt; or earlier.';
106+
$this->expectExceptionMessage($errorString);
107+
108+
/** @var RequestInterface|MockObject $request */
109+
$request = $this->createMock('\Sabre\HTTP\RequestInterface');
110+
$request
59111
->expects($this->once())
60-
->method('getSystemValue')
61-
->with('minimum.supported.desktop.version', '2.3.0')
62-
->willReturn('1.7.0');
112+
->method('getHeader')
113+
->with('User-Agent')
114+
->willReturn($userAgent);
63115

64116
$this->blockLegacyClientVersionPlugin->beforeHandler($request);
65117
}
66118

67-
public function newAndAlternateDesktopClientProvider(): array {
119+
public static function newAndAlternateDesktopClientProvider(): array {
68120
return [
69121
['Mozilla/5.0 (Windows) mirall/1.7.0'],
70122
['Mozilla/5.0 (Bogus Text) mirall/1.9.3'],
71123
['Mozilla/5.0 (Not Our Client But Old Version) LegacySync/1.1.0'],
124+
['Mozilla/5.0 (Windows) mirall/4.7.0'],
125+
['Mozilla/5.0 (Bogus Text) mirall/3.9.3'],
126+
['Mozilla/5.0 (Not Our Client But Old Version) LegacySync/45.0.0'],
72127
];
73128
}
74129

@@ -85,10 +140,14 @@ public function testBeforeHandlerSuccess(string $userAgent): void {
85140
->willReturn($userAgent);
86141

87142
$this->config
88-
->expects($this->once())
89-
->method('getSystemValue')
90-
->with('minimum.supported.desktop.version', '2.3.0')
91-
->willReturn('1.7.0');
143+
->expects($this->exactly(2))
144+
->method('getSystemValueString')
145+
->willReturnCallback(function (string $key) {
146+
if ($key === 'minimum.supported.desktop.version') {
147+
return '1.7.0';
148+
}
149+
return '10.0.0';
150+
});
92151

93152
$this->blockLegacyClientVersionPlugin->beforeHandler($request);
94153
}
@@ -101,6 +160,7 @@ public function testBeforeHandlerNoUserAgent(): void {
101160
->method('getHeader')
102161
->with('User-Agent')
103162
->willReturn(null);
163+
104164
$this->blockLegacyClientVersionPlugin->beforeHandler($request);
105165
}
106166
}

config/config.sample.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2141,6 +2141,15 @@
21412141
*/
21422142
'minimum.supported.desktop.version' => '2.3.0',
21432143

2144+
/**
2145+
* The maximum Nextcloud desktop client version that will be allowed to sync with
2146+
* this server instance. All connections made from later clients will be denied
2147+
* by the server.
2148+
*
2149+
* Defaults to 99.99.99
2150+
*/
2151+
'maximum.supported.desktop.version' => '99.99.99',
2152+
21442153
/**
21452154
* Option to allow local storage to contain symlinks.
21462155
* WARNING: Not recommended. This would make it possible for Nextcloud to access

0 commit comments

Comments
 (0)