Merge pull request #31250 from nextcloud/backport/30114/stable20

[stable20] Prevent writing invalid mtime

Author: blizzz

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

@@ -414,6 +414,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;
 	}
 }

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

@@ -354,28 +354,28 @@ public function legalMtimeProvider() {
 				'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
+				'expected result' => null
 			],
 			"zero string" => [
 				'HTTP_X_OC_MTIME' => "0",
-				'expected result' => 0
+				'expected result' => null
 			],
 			"negative zero string" => [
 				'HTTP_X_OC_MTIME' => "-0",
-				'expected result' => 0
+				'expected result' => null
 			],
 			"string starting with number following by char" => [
 				'HTTP_X_OC_MTIME' => "2345asdf",
@@ -391,11 +391,11 @@ public function legalMtimeProvider() {
 			],
 			"negative int" => [
 				'HTTP_X_OC_MTIME' => -34,
-				'expected result' => -34
+				'expected result' => null
 			],
 			"negative float" => [
 				'HTTP_X_OC_MTIME' => -34.43,
-				'expected result' => -34
+				'expected result' => null
 			],
 		];
 	}
@@ -414,7 +414,6 @@ public function testPutSingleFileLegalMtime($requestMtime, $resultMtime) {
 
 		if ($resultMtime === null) {
 			$this->expectException(\InvalidArgumentException::class);
-			$this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp).");
 		}
 
 		$this->doPut($file, null, $request);
@@ -440,7 +439,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);

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

@@ -165,8 +165,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]);
+	}
 }