Merge pull request #7313 from nextcloud/ensure-that-x-oc-mtime-header-is-an-integer-with-chunked-uploads

Ensure that X-OC-MTime header is an integer with chunked uploads

1 files changed, 51 insertions, 16 deletions

diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php
index f172bde5f1f..32cc8b7adeb 100644
--- a/apps/dav/lib/Connector/Sabre/File.php
+++ b/apps/dav/lib/Connector/Sabre/File.php
@@ -36,13 +36,16 @@
 
 namespace OCA\DAV\Connector\Sabre;
 
+use OC\AppFramework\Http\Request;
 use OC\Files\Filesystem;
+use OC\Files\View;
 use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge;
 use OCA\DAV\Connector\Sabre\Exception\FileLocked;
 use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
 use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
 use OCP\Files\EntityTooLargeException;
+use OCP\Files\FileInfo;
 use OCP\Files\ForbiddenException;
 use OCP\Files\InvalidContentException;
 use OCP\Files\InvalidPathException;
@@ -51,6 +54,7 @@ use OCP\Files\NotPermittedException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
+use OCP\Share\IManager;
 use Sabre\DAV\Exception;
 use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\Forbidden;
@@ -61,6 +65,26 @@ use Sabre\DAV\Exception\NotFound;
 
 class File extends Node implements IFile {
 
+	protected $request;
+
+	/**
+	 * Sets up the node, expects a full path name
+	 *
+	 * @param \OC\Files\View $view
+	 * @param \OCP\Files\FileInfo $info
+	 * @param \OCP\Share\IManager $shareManager
+	 * @param \OC\AppFramework\Http\Request $request
+	 */
+	public function __construct(View $view, FileInfo $info, IManager $shareManager = null, Request $request = null) {
+		parent::__construct($view, $info, $shareManager);
+
+		if (isset($request)) {
+			$this->request = $request;
+		} else {
+			$this->request = \OC::$server->getRequest();
+		}
+	}
+
 	/**
 	 * Updates the data
 	 *
@@ -208,15 +232,10 @@ class File extends Node implements IFile {
 			}
 
 			// allow sync clients to send the mtime along in a header
-			$request = \OC::$server->getRequest();
-			if (isset($request->server['HTTP_X_OC_MTIME'])) {
-				$mtimeStr = $request->server['HTTP_X_OC_MTIME'];
-				if (!is_numeric($mtimeStr)) {
-					throw new \InvalidArgumentException('X-OC-Mtime header must be an integer (unix timestamp).');
-				}
-				$mtime = intval($mtimeStr);
+			if (isset($this->request->server['HTTP_X_OC_MTIME'])) {
+				$mtime = $this->sanitizeMtime($this->request->server['HTTP_X_OC_MTIME']);
 				if ($this->fileView->touch($this->path, $mtime)) {
-					header('X-OC-MTime: accepted');
+					$this->header('X-OC-MTime: accepted');
 				}
 			}
 					
@@ -226,8 +245,8 @@ class File extends Node implements IFile {
 
 			$this->refreshInfo();
 
-			if (isset($request->server['HTTP_OC_CHECKSUM'])) {
-				$checksum = trim($request->server['HTTP_OC_CHECKSUM']);
+			if (isset($this->request->server['HTTP_OC_CHECKSUM'])) {
+				$checksum = trim($this->request->server['HTTP_OC_CHECKSUM']);
 				$this->fileView->putFileInfo($this->path, ['checksum' => $checksum]);
 				$this->refreshInfo();
 			} else if ($this->getChecksum() !== null && $this->getChecksum() !== '') {
@@ -470,10 +489,10 @@ class File extends Node implements IFile {
 				}
 
 				// allow sync clients to send the mtime along in a header
-				$request = \OC::$server->getRequest();
-				if (isset($request->server['HTTP_X_OC_MTIME'])) {
-					if ($targetStorage->touch($targetInternalPath, $request->server['HTTP_X_OC_MTIME'])) {
-						header('X-OC-MTime: accepted');
+				if (isset($this->request->server['HTTP_X_OC_MTIME'])) {
+					$mtime = $this->sanitizeMtime($this->request->server['HTTP_X_OC_MTIME']);
+					if ($targetStorage->touch($targetInternalPath, $mtime)) {
+						$this->header('X-OC-MTime: accepted');
 					}
 				}
 
@@ -487,8 +506,8 @@ class File extends Node implements IFile {
 				// FIXME: should call refreshInfo but can't because $this->path is not the of the final file
 				$info = $this->fileView->getFileInfo($targetPath);
 
-				if (isset($request->server['HTTP_OC_CHECKSUM'])) {
-					$checksum = trim($request->server['HTTP_OC_CHECKSUM']);
+				if (isset($this->request->server['HTTP_OC_CHECKSUM'])) {
+					$checksum = trim($this->request->server['HTTP_OC_CHECKSUM']);
 					$this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]);
 				} else if ($info->getChecksum() !== null && $info->getChecksum() !== '') {
 					$this->fileView->putFileInfo($this->path, ['checksum' => '']);
@@ -570,6 +589,18 @@ class File extends Node implements IFile {
 		throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e);
 	}
 
+	private function sanitizeMtime($mtimeFromRequest) {
+		// In PHP 5.X "is_numeric" returns true for strings in hexadecimal
+		// notation. This is no longer the case in PHP 7.X, so this check
+		// ensures that strings with hexadecimal notations fail too in PHP 5.X.
+		$isHexadecimal = is_string($mtimeFromRequest) && preg_match('/^\s*0[xX]/', $mtimeFromRequest);
+		if ($isHexadecimal || !is_numeric($mtimeFromRequest)) {
+			throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).');
+		}
+
+		return intval($mtimeFromRequest);
+	}
+
 	/**
 	 * Get the checksum for this file
 	 *
@@ -578,4 +609,8 @@ class File extends Node implements IFile {
 	public function getChecksum() {
 		return $this->info->getChecksum();
 	}
+
+	protected function header($string) {
+		\header($string);
+	}
 }