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 b83e5cbcbc1b571308dce8b9e8c01ff929432a8f..a5bcfce4c908d3995c07894f5b6f677914d779f7 100644 (file)
--- 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 904faa8e8c3dbf0b2592bd7008600293eb5e5731..5e80d0613bff8ba486f3172a8aeabccc72e87489 100644 (file)
--- 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 4244792c46a32415e5c1046aee9a0484c6d7b846..0d9ef1c7f92552e5989dca639c8a5a68d3a330a9 100644 (file)
--- 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]);
+       }
 }