Merge pull request #31249 from nextcloud/backport/30114/stable21

[stable21] Prevent writing invalid mtime

3 files changed, 63 insertions, 14 deletions

diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php
index b83e5cbcbc1..a5bcfce4c90 100644
--- a/apps/dav/lib/Connector/Sabre/Node.php
+++ b/apps/dav/lib/Connector/Sabre/Node.php
@@ -414,6 +414,11 @@ abstract class Node implements \Sabre\DAV\INode {
 			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 904faa8e8c3..5e80d0613bf 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
@@ -362,28 +362,28 @@ class FileTest extends TestCase {
 				'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",
@@ -399,11 +399,11 @@ class FileTest extends TestCase {
 			],
 			"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
 			],
 		];
 	}
@@ -422,7 +422,6 @@ class FileTest extends TestCase {
 
 		if ($resultMtime === null) {
 			$this->expectException(\InvalidArgumentException::class);
-			$this->expectExceptionMessage("X-OC-MTime header must be an integer (unix timestamp).");
 		}
 
 		$this->doPut($file, null, $request);
@@ -448,7 +447,6 @@ class FileTest extends TestCase {
 
 		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);
diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
index 4244792c46a..0d9ef1c7f92 100644
--- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
@@ -165,8 +165,54 @@ class NodeTest extends \Test\TestCase {
 			->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]);
+	}
 }