Skip to content

Commit 8b71f45

Browse files
skjnldsvPVince81
skjnldsv
authored and
PVince81
committed
Prevent writing invalid mtime
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
1 parent de83b4b commit 8b71f45

3 files changed

Lines changed: 87 additions & 38 deletions

File tree

‎apps/dav/lib/Connector/Sabre/Node.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,11 @@ protected function sanitizeMtime($mtimeFromRequest) {
415415
throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).');
416416
}
417417

418+
// Prevent writing invalid mtime (timezone-proof)
419+
if ((int)$mtimeFromRequest <= 24 * 60 * 60) {
420+
throw new \InvalidArgumentException('X-OC-MTime header must be a valid positive integer');
421+
}
422+
418423
return (int)$mtimeFromRequest;
419424
}
420425

‎apps/dav/tests/unit/Connector/Sabre/FileTest.php‎

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -349,52 +349,52 @@ public function testPutSingleFile() {
349349
public function legalMtimeProvider() {
350350
return [
351351
"string" => [
352-
'HTTP_X_OC_MTIME' => "string",
353-
'expected result' => null
352+
'HTTP_X_OC_MTIME' => "string",
353+
'expected result' => null
354354
],
355355
"castable string (int)" => [
356-
'HTTP_X_OC_MTIME' => "34",
357-
'expected result' => 34
356+
'HTTP_X_OC_MTIME' => "987654321",
357+
'expected result' => 987654321
358358
],
359359
"castable string (float)" => [
360-
'HTTP_X_OC_MTIME' => "34.56",
361-
'expected result' => 34
360+
'HTTP_X_OC_MTIME' => "123456789.56",
361+
'expected result' => 123456789
362362
],
363363
"float" => [
364-
'HTTP_X_OC_MTIME' => 34.56,
365-
'expected result' => 34
364+
'HTTP_X_OC_MTIME' => 123456789.56,
365+
'expected result' => 123456789
366366
],
367367
"zero" => [
368-
'HTTP_X_OC_MTIME' => 0,
369-
'expected result' => 0
368+
'HTTP_X_OC_MTIME' => 0,
369+
'expected result' => null
370370
],
371371
"zero string" => [
372-
'HTTP_X_OC_MTIME' => "0",
373-
'expected result' => 0
372+
'HTTP_X_OC_MTIME' => "0",
373+
'expected result' => null
374374
],
375375
"negative zero string" => [
376-
'HTTP_X_OC_MTIME' => "-0",
377-
'expected result' => 0
376+
'HTTP_X_OC_MTIME' => "-0",
377+
'expected result' => null
378378
],
379379
"string starting with number following by char" => [
380-
'HTTP_X_OC_MTIME' => "2345asdf",
381-
'expected result' => null
380+
'HTTP_X_OC_MTIME' => "2345asdf",
381+
'expected result' => null
382382
],
383383
"string castable hex int" => [
384-
'HTTP_X_OC_MTIME' => "0x45adf",
385-
'expected result' => null
384+
'HTTP_X_OC_MTIME' => "0x45adf",
385+
'expected result' => null
386386
],
387387
"string that looks like invalid hex int" => [
388-
'HTTP_X_OC_MTIME' => "0x123g",
389-
'expected result' => null
388+
'HTTP_X_OC_MTIME' => "0x123g",
389+
'expected result' => null
390390
],
391391
"negative int" => [
392-
'HTTP_X_OC_MTIME' => -34,
393-
'expected result' => -34
392+
'HTTP_X_OC_MTIME' => -34,
393+
'expected result' => null
394394
],
395395
"negative float" => [
396-
'HTTP_X_OC_MTIME' => -34.43,
397-
'expected result' => -34
396+
'HTTP_X_OC_MTIME' => -34.43,
397+
'expected result' => null
398398
],
399399
];
400400
}
@@ -405,15 +405,14 @@ public function legalMtimeProvider() {
405405
*/
406406
public function testPutSingleFileLegalMtime($requestMtime, $resultMtime) {
407407
$request = new \OC\AppFramework\Http\Request([
408-
'server' => [
409-
'HTTP_X_OC_MTIME' => $requestMtime,
410-
]
408+
'server' => [
409+
'HTTP_X_OC_MTIME' => $requestMtime,
410+
]
411411
], null, $this->config, null);
412412
$file = 'foo.txt';
413413

414414
if ($resultMtime === null) {
415415
$this->expectException(\InvalidArgumentException::class);
416-
$this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp).");
417416
}
418417

419418
$this->doPut($file, null, $request);
@@ -439,7 +438,6 @@ public function testChunkedPutLegalMtime($requestMtime, $resultMtime) {
439438

440439
if ($resultMtime === null) {
441440
$this->expectException(\Sabre\DAV\Exception::class);
442-
$this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp).");
443441
}
444442

445443
$this->doPut($file.'-chunking-12345-2-0', null, $request);
@@ -836,7 +834,7 @@ public function testSetNameInvalidChars() {
836834
$file->setName('/super*star.txt');
837835
}
838836

839-
837+
840838
public function testUploadAbort() {
841839
// setup
842840
$view = $this->getMockBuilder(View::class)
@@ -880,7 +878,7 @@ public function testUploadAbort() {
880878
$this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files');
881879
}
882880

883-
881+
884882
public function testDeleteWhenAllowed() {
885883
// setup
886884
$view = $this->getMockBuilder(View::class)
@@ -900,7 +898,7 @@ public function testDeleteWhenAllowed() {
900898
$file->delete();
901899
}
902900

903-
901+
904902
public function testDeleteThrowsWhenDeletionNotAllowed() {
905903
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
906904

@@ -918,7 +916,7 @@ public function testDeleteThrowsWhenDeletionNotAllowed() {
918916
$file->delete();
919917
}
920918

921-
919+
922920
public function testDeleteThrowsWhenDeletionFailed() {
923921
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
924922

@@ -941,7 +939,7 @@ public function testDeleteThrowsWhenDeletionFailed() {
941939
$file->delete();
942940
}
943941

944-
942+
945943
public function testDeleteThrowsWhenDeletionThrows() {
946944
$this->expectException(\OCA\DAV\Connector\Sabre\Exception\Forbidden::class);
947945

@@ -1111,7 +1109,7 @@ private function getFileInfos($path = '', View $userView = null) {
11111109
];
11121110
}
11131111

1114-
1112+
11151113
public function testGetFopenFails() {
11161114
$this->expectException(\Sabre\DAV\Exception\ServiceUnavailable::class);
11171115

@@ -1131,7 +1129,7 @@ public function testGetFopenFails() {
11311129
$file->get();
11321130
}
11331131

1134-
1132+
11351133
public function testGetFopenThrows() {
11361134
$this->expectException(\OCA\DAV\Connector\Sabre\Exception\Forbidden::class);
11371135

@@ -1151,7 +1149,7 @@ public function testGetFopenThrows() {
11511149
$file->get();
11521150
}
11531151

1154-
1152+
11551153
public function testGetThrowsIfNoPermission() {
11561154
$this->expectException(\Sabre\DAV\Exception\NotFound::class);
11571155

‎apps/dav/tests/unit/Connector/Sabre/NodeTest.php‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,54 @@ public function testSharePermissions($type, $user, $permissions, $expected) {
164164
->disableOriginalConstructor()
165165
->getMock();
166166

167-
$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
167+
$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
168168
$this->invokePrivate($node, 'shareManager', [$shareManager]);
169169
$this->assertEquals($expected, $node->getSharePermissions($user));
170170
}
171+
172+
public function sanitizeMtimeProvider() {
173+
return [
174+
[123456789, 123456789],
175+
['987654321', 987654321],
176+
];
177+
}
178+
179+
/**
180+
* @dataProvider sanitizeMtimeProvider
181+
*/
182+
public function testSanitizeMtime($mtime, $expected) {
183+
$view = $this->getMockBuilder(View::class)
184+
->disableOriginalConstructor()
185+
->getMock();
186+
$info = $this->getMockBuilder(FileInfo::class)
187+
->disableOriginalConstructor()
188+
->getMock();
189+
190+
$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
191+
$result = $this->invokePrivate($node, 'sanitizeMtime', [$mtime]);
192+
$this->assertEquals($expected, $result);
193+
}
194+
195+
public function invalidSanitizeMtimeProvider() {
196+
return [
197+
[-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1]
198+
];
199+
}
200+
201+
/**
202+
* @dataProvider invalidSanitizeMtimeProvider
203+
*/
204+
public function testInvalidSanitizeMtime($mtime) {
205+
$this->expectException(\InvalidArgumentException::class);
206+
207+
$view = $this->getMockBuilder(View::class)
208+
->disableOriginalConstructor()
209+
->getMock();
210+
$info = $this->getMockBuilder(FileInfo::class)
211+
->disableOriginalConstructor()
212+
->getMock();
213+
214+
$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
215+
$result = $this->invokePrivate($node, 'sanitizeMtime', [$mtime]);
216+
}
171217
}

0 commit comments

Comments
 (0)