Prevent writing invalid mtime

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>

diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php
index aa03f42dc35c0dbed9c24ec94a0a138080b23325..3d7ffafadc2e2ff2ee686270e26978e58cd81247 100644 (file)
--- a/apps/dav/lib/Connector/Sabre/Node.php
+++ b/apps/dav/lib/Connector/Sabre/Node.php
@@ -412,6 +412,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 304e8abed071e3dce526bbebf400a1b7e6cc00d6..3e6a47d5854f1a25b30d3d8bb267ff9bebcd28ea 100644 (file)
--- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
@@ -361,28 +361,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",
@@ -398,11 +398,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
                        ],
                ];
        }
@@ -421,7 +421,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);
@@ -447,7 +446,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 18ccd0fe69073a09da6eee55b01243a1e489aaba..00fd0ebd8aa2ab4635891aa9d55353a9a04bc8eb 100644 (file)
--- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
@@ -164,8 +164,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]);
+       }
 }