From f69765a42ac81184d1a30faa522158257f71ce47 Mon Sep 17 00:00:00 2001
From: Varun Patil <radialapps@gmail.com>
Date: Thu, 19 Oct 2023 18:47:58 -0700
Subject: [PATCH] fs: improve sanitization

Signed-off-by: Varun Patil <radialapps@gmail.com>
---
 lib/Controller/ArchiveController.php | 14 +++++++++++---
 lib/Controller/PublicController.php  |  3 ++-
 lib/Db/FsManager.php                 | 16 +++++++++-------
 lib/Util.php                         | 19 +++++++++++++------
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/lib/Controller/ArchiveController.php b/lib/Controller/ArchiveController.php
index a002a407..e09771ac 100644
--- a/lib/Controller/ArchiveController.php
+++ b/lib/Controller/ArchiveController.php
@@ -72,6 +72,7 @@ class ArchiveController extends GenericApiController
             $fileStorageId = $file->getStorage()->getId();
             $parent = $file->getParent();
             $isArchived = false;
+            $depth = 0;
             while (true) {
                 /** @psalm-suppress DocblockTypeContradiction */
                 if (null === $parent) {
@@ -98,12 +99,17 @@ class ArchiveController extends GenericApiController
                 }
 
                 // Hit an archive folder root
-                if ($parent->getName() === \OCA\Memories\Util::$ARCHIVE_FOLDER) {
+                if (Util::ARCHIVE_FOLDER === $parent->getName()) {
                     $isArchived = true;
 
                     break;
                 }
 
+                // Too deep
+                if (++$depth > 32) {
+                    throw new \Exception('[Archive] Max recursion depth exceeded');
+                }
+
                 $parent = $parent->getParent();
             }
 
@@ -133,8 +139,10 @@ class ArchiveController extends GenericApiController
                 $parent = $parent->getParent();
             } else {
                 // file not in archive, put it in there
-                $af = \OCA\Memories\Util::$ARCHIVE_FOLDER;
-                $destinationPath = Util::sanitizePath($af.$relativeFilePath);
+                $destinationPath = Util::sanitizePath(Util::ARCHIVE_FOLDER.$relativeFilePath);
+                if (null === $destinationPath) {
+                    throw Exceptions::BadRequest('Invalid archive destination path');
+                }
             }
 
             // Remove the filename
diff --git a/lib/Controller/PublicController.php b/lib/Controller/PublicController.php
index 422224d9..3ba4660d 100644
--- a/lib/Controller/PublicController.php
+++ b/lib/Controller/PublicController.php
@@ -189,10 +189,11 @@ class PublicController extends AuthPublicShareController
         $foldersPath = Util::sanitizePath('/'.$foldersPath.'/');
 
         // Check if relPath starts with foldersPath
-        if (!str_starts_with($relPath, $foldersPath)) {
+        if (empty($foldersPath) || !str_starts_with($relPath, $foldersPath)) {
             return;
         }
 
+        /** @var string $foldersPath */
         // Remove foldersPath from start of relPath
         $relPath = substr($relPath, \strlen($foldersPath));
 
diff --git a/lib/Db/FsManager.php b/lib/Db/FsManager.php
index 582c0e82..808d6590 100644
--- a/lib/Db/FsManager.php
+++ b/lib/Db/FsManager.php
@@ -114,14 +114,16 @@ class FsManager
 
         try {
             foreach ($paths as $path) {
-                $node = $userFolder->get(Util::sanitizePath($path));
-                $root->addFolder($node);
-                $etag .= $node->getEtag();
+                if ($sanitized = Util::sanitizePath($path)) {
+                    $node = $userFolder->get($sanitized);
+                    $root->addFolder($node);
+                    $etag .= $node->getEtag();
+                } else {
+                    throw new \Exception("invalid path {$path}");
+                }
             }
-        } catch (\OCP\Files\NotFoundException $e) {
-            $msg = $e->getMessage();
-
-            throw new \Exception("Folder not found: {$msg}");
+        } catch (\Exception $e) {
+            throw new \Exception("Folder not found: {$e->getMessage()}");
         }
 
         // Add shares or external stores inside the current folders
diff --git a/lib/Util.php b/lib/Util.php
index 7e1415d7..c4b5c3ab 100644
--- a/lib/Util.php
+++ b/lib/Util.php
@@ -18,7 +18,7 @@ class Util
 {
     use UtilController;
 
-    public static string $ARCHIVE_FOLDER = '.archive';
+    public const ARCHIVE_FOLDER = '.archive';
 
     /**
      * Get host CPU architecture (amd64 or aarch64).
@@ -336,20 +336,27 @@ class Util
                 ?: self::getSystemConfig('memories.timeline.default_path');
 
         return array_map(
-            static fn ($p) => self::sanitizePath(trim($p)),
+            static fn ($path) => self::sanitizePath(trim($path))
+                ?? throw new \InvalidArgumentException("Invalid timeline path: {$path}"),
             explode(';', $paths),
         );
     }
 
     /**
      * Sanitize a path to keep only ASCII characters and special characters.
-     * Blank will be returned on error.
+     * Null will be returned on error.
      */
-    public static function sanitizePath(string $path): string
+    public static function sanitizePath(string $path): ?string
     {
-        $path = str_replace("\0", '', $path); // remove null characters
+        // remove double slashes and such
+        $normalized = \OC\Files\Filesystem::normalizePath($path, false);
 
-        return mb_ereg_replace('\/\/+', '/', $path) ?: ''; // remove extra slashes
+        // look for invalid characters and pattern
+        if (!\OC\Files\Filesystem::isValidPath($normalized)) {
+            return null;
+        }
+
+        return $normalized;
     }
 
     /**