diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index f178b2fbfbdfe..c0ffa1d0a97b4 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -415,6 +415,11 @@ protected function sanitizeMtime($mtimeFromRequest) { throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); } + // Prevent writing invalid mtime (timezone-proof) + if ((int)$mtimeFromRequest <= 24 * 60 * 60) { + throw new \InvalidArgumentException('X-OC-MTime header must be a valid positive integer'); + } + return (int)$mtimeFromRequest; } diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 8842d8be31653..e6e13e8934462 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -349,52 +349,52 @@ public function testPutSingleFile() { public function legalMtimeProvider() { return [ "string" => [ - 'HTTP_X_OC_MTIME' => "string", - 'expected result' => null + 'HTTP_X_OC_MTIME' => "string", + 'expected result' => null ], "castable string (int)" => [ - 'HTTP_X_OC_MTIME' => "34", - 'expected result' => 34 + 'HTTP_X_OC_MTIME' => "987654321", + 'expected result' => 987654321 ], "castable string (float)" => [ - 'HTTP_X_OC_MTIME' => "34.56", - 'expected result' => 34 + 'HTTP_X_OC_MTIME' => "123456789.56", + 'expected result' => 123456789 ], "float" => [ - 'HTTP_X_OC_MTIME' => 34.56, - 'expected result' => 34 + 'HTTP_X_OC_MTIME' => 123456789.56, + 'expected result' => 123456789 ], "zero" => [ - 'HTTP_X_OC_MTIME' => 0, - 'expected result' => 0 + 'HTTP_X_OC_MTIME' => 0, + 'expected result' => null ], "zero string" => [ - 'HTTP_X_OC_MTIME' => "0", - 'expected result' => 0 + 'HTTP_X_OC_MTIME' => "0", + 'expected result' => null ], "negative zero string" => [ - 'HTTP_X_OC_MTIME' => "-0", - 'expected result' => 0 + 'HTTP_X_OC_MTIME' => "-0", + 'expected result' => null ], "string starting with number following by char" => [ - 'HTTP_X_OC_MTIME' => "2345asdf", - 'expected result' => null + 'HTTP_X_OC_MTIME' => "2345asdf", + 'expected result' => null ], "string castable hex int" => [ - 'HTTP_X_OC_MTIME' => "0x45adf", - 'expected result' => null + 'HTTP_X_OC_MTIME' => "0x45adf", + 'expected result' => null ], "string that looks like invalid hex int" => [ - 'HTTP_X_OC_MTIME' => "0x123g", - 'expected result' => null + 'HTTP_X_OC_MTIME' => "0x123g", + 'expected result' => null ], "negative int" => [ - 'HTTP_X_OC_MTIME' => -34, - 'expected result' => -34 + 'HTTP_X_OC_MTIME' => -34, + 'expected result' => null ], "negative float" => [ - 'HTTP_X_OC_MTIME' => -34.43, - 'expected result' => -34 + 'HTTP_X_OC_MTIME' => -34.43, + 'expected result' => null ], ]; } @@ -405,15 +405,14 @@ public function legalMtimeProvider() { */ public function testPutSingleFileLegalMtime($requestMtime, $resultMtime) { $request = new \OC\AppFramework\Http\Request([ - 'server' => [ - 'HTTP_X_OC_MTIME' => $requestMtime, - ] + 'server' => [ + 'HTTP_X_OC_MTIME' => $requestMtime, + ] ], null, $this->config, null); $file = 'foo.txt'; if ($resultMtime === null) { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp)."); } $this->doPut($file, null, $request); @@ -439,7 +438,6 @@ public function testChunkedPutLegalMtime($requestMtime, $resultMtime) { if ($resultMtime === null) { $this->expectException(\Sabre\DAV\Exception::class); - $this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp)."); } $this->doPut($file.'-chunking-12345-2-0', null, $request); @@ -836,7 +834,7 @@ public function testSetNameInvalidChars() { $file->setName('/super*star.txt'); } - + public function testUploadAbort() { // setup $view = $this->getMockBuilder(View::class) @@ -880,7 +878,7 @@ public function testUploadAbort() { $this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files'); } - + public function testDeleteWhenAllowed() { // setup $view = $this->getMockBuilder(View::class) @@ -900,7 +898,7 @@ public function testDeleteWhenAllowed() { $file->delete(); } - + public function testDeleteThrowsWhenDeletionNotAllowed() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -918,7 +916,7 @@ public function testDeleteThrowsWhenDeletionNotAllowed() { $file->delete(); } - + public function testDeleteThrowsWhenDeletionFailed() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -941,7 +939,7 @@ public function testDeleteThrowsWhenDeletionFailed() { $file->delete(); } - + public function testDeleteThrowsWhenDeletionThrows() { $this->expectException(\OCA\DAV\Connector\Sabre\Exception\Forbidden::class); @@ -1111,7 +1109,7 @@ private function getFileInfos($path = '', View $userView = null) { ]; } - + public function testGetFopenFails() { $this->expectException(\Sabre\DAV\Exception\ServiceUnavailable::class); @@ -1131,7 +1129,7 @@ public function testGetFopenFails() { $file->get(); } - + public function testGetFopenThrows() { $this->expectException(\OCA\DAV\Connector\Sabre\Exception\Forbidden::class); @@ -1151,7 +1149,7 @@ public function testGetFopenThrows() { $file->get(); } - + public function testGetThrowsIfNoPermission() { $this->expectException(\Sabre\DAV\Exception\NotFound::class); diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php index f0991d05992b1..da635aba27f56 100644 --- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php @@ -164,8 +164,54 @@ public function testSharePermissions($type, $user, $permissions, $expected) { ->disableOriginalConstructor() ->getMock(); - $node = new \OCA\DAV\Connector\Sabre\File($view, $info); + $node = new \OCA\DAV\Connector\Sabre\File($view, $info); $this->invokePrivate($node, 'shareManager', [$shareManager]); $this->assertEquals($expected, $node->getSharePermissions($user)); } + + public function sanitizeMtimeProvider() { + return [ + [123456789, 123456789], + ['987654321', 987654321], + ]; + } + + /** + * @dataProvider sanitizeMtimeProvider + */ + public function testSanitizeMtime($mtime, $expected) { + $view = $this->getMockBuilder(View::class) + ->disableOriginalConstructor() + ->getMock(); + $info = $this->getMockBuilder(FileInfo::class) + ->disableOriginalConstructor() + ->getMock(); + + $node = new \OCA\DAV\Connector\Sabre\File($view, $info); + $result = $this->invokePrivate($node, 'sanitizeMtime', [$mtime]); + $this->assertEquals($expected, $result); + } + + public function invalidSanitizeMtimeProvider() { + return [ + [-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1] + ]; + } + + /** + * @dataProvider invalidSanitizeMtimeProvider + */ + public function testInvalidSanitizeMtime($mtime) { + $this->expectException(\InvalidArgumentException::class); + + $view = $this->getMockBuilder(View::class) + ->disableOriginalConstructor() + ->getMock(); + $info = $this->getMockBuilder(FileInfo::class) + ->disableOriginalConstructor() + ->getMock(); + + $node = new \OCA\DAV\Connector\Sabre\File($view, $info); + $result = $this->invokePrivate($node, 'sanitizeMtime', [$mtime]); + } }