Improve path validation for repo-* commands.

Check for invalid path in repo-* commands. Perform path validation and throw an error when appropriate. Path may not contain '//'. Strip trailing '/' from path. Absolute path must fall under repo path.

doc/xml/release.xml

@@ -42,6 +42,17 @@
                         <p>Increase precision of percent complete logging for <cmd>backup</cmd> and <cmd>restore</cmd>.</p>
                     </release-item>
 
+                    <release-item>
+                        <github-pull-request id="1697"/>
+
+                        <release-item-contributor-list>
+                            <release-item-contributor id="reid.thompson"/>
+                            <release-item-reviewer id="david.steele"/>
+                        </release-item-contributor-list>
+
+                        <p>Improve path validation for <cmd>repo-*</cmd> commands.</p>
+                    </release-item>
+
                     <release-item>
                         <github-issue id="1608"/>
                         <github-pull-request id="1676"/>

src/Makefile.in

@@ -70,6 +70,7 @@ SRCS = \
 	command/control/start.c \
 	command/control/stop.c \
 	command/local/local.c \
+	command/repo/common.c \
 	command/repo/create.c \
 	command/repo/get.c \
 	command/repo/ls.c \

src/command/repo/common.c

@@ -0,0 +1,63 @@
+/***********************************************************************************************************************************
+Common Functions and Definitions for Repo Commands
+***********************************************************************************************************************************/
+#include "build.auto.h"
+
+#include "command/repo/common.h"
+#include "common/debug.h"
+#include "config/config.h"
+
+/**********************************************************************************************************************************/
+String *
+repoPathIsValid(const String *path)
+{
+    FUNCTION_LOG_BEGIN(logLevelDebug);
+        FUNCTION_LOG_PARAM(STRING, path);
+    FUNCTION_LOG_END();
+
+    ASSERT(path != NULL);
+
+    String *result = NULL;
+
+    MEM_CONTEXT_TEMP_BEGIN()
+    {
+        // Make sure there are no occurrences of //
+        if (strstr(strZ(path), "//") != NULL)
+            THROW_FMT(ParamInvalidError, "path '%s' cannot contain //", strZ(path));
+
+        // Make sure the path does not end with a slash
+        if (strEndsWith(path, FSLASH_STR))
+            path = strPath(path);
+
+        // Validate absolute paths
+        if (strBeginsWith(path, FSLASH_STR))
+        {
+            // Check that the file path begins with the repo path
+            if (!strBeginsWith(path, cfgOptionStr(cfgOptRepoPath)))
+            {
+                THROW_FMT(
+                    ParamInvalidError, "absolute path '%s' is not in base path '%s'", strZ(path),
+                    strZ(cfgOptionDisplay(cfgOptRepoPath)));
+            }
+
+            MEM_CONTEXT_PRIOR_BEGIN()
+            {
+                // Get the relative part of the file
+                result = strSub(
+                    path, strEq(cfgOptionStr(cfgOptRepoPath), FSLASH_STR) ? 1 : strSize(cfgOptionStr(cfgOptRepoPath)) + 1);
+            }
+            MEM_CONTEXT_PRIOR_END();
+        }
+        else
+        {
+            MEM_CONTEXT_PRIOR_BEGIN()
+            {
+                result = strDup(path);
+            }
+            MEM_CONTEXT_PRIOR_END();
+        }
+    }
+    MEM_CONTEXT_TEMP_END();
+
+    FUNCTION_LOG_RETURN(STRING, result);
+}

src/command/repo/common.h

@@ -0,0 +1,15 @@
+/***********************************************************************************************************************************
+Repo Command Common
+***********************************************************************************************************************************/
+#ifndef COMMAND_REPO_COMMON_H
+#define COMMAND_REPO_COMMON_H
+
+#include "common/type/string.h"
+
+/***********************************************************************************************************************************
+Functions and Definitions
+***********************************************************************************************************************************/
+// Path cannot contain //. Strip trailing /. Absolute path must fall under repo path. Throw error or return validated relative path.
+String *repoPathIsValid(const String *path);
+
+#endif

src/command/repo/get.c

@@ -5,6 +5,7 @@ Repository Get Command
 
 #include <unistd.h>
 
+#include "command/repo/common.h"
 #include "common/crypto/cipherBlock.h"
 #include "common/debug.h"
 #include "common/io/fdWrite.h"
@@ -40,20 +41,8 @@ storageGetProcess(IoWrite *destination)
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
-        // If the source path is absolute then get the relative part of the file
+        // Is path valid for repo?
-        if (strBeginsWith(file, FSLASH_STR))
+        file = repoPathIsValid(file);
-        {
-            // Check that the file path begins with the repo path
-            if (!strBeginsWith(file, cfgOptionStr(cfgOptRepoPath)))
-            {
-                THROW_FMT(
-                    OptionInvalidValueError, "absolute path '%s' is not in base path '%s'", strZ(file),
-                    strZ(cfgOptionDisplay(cfgOptRepoPath)));
-            }
-
-            // Get the relative part of the file
-            file = strSub(file, strEq(cfgOptionStr(cfgOptRepoPath), FSLASH_STR) ? 1 : strSize(cfgOptionStr(cfgOptRepoPath)) + 1);
-        }
 
         // Create new file read
         IoRead *source = storageReadIo(storageNewReadP(storageRepo(), file, .ignoreMissing = cfgOptionBool(cfgOptIgnoreMissing)));

src/command/repo/ls.c

@@ -5,6 +5,7 @@ Repository List Command
 
 #include <unistd.h>
 
+#include "command/repo/common.h"
 #include "common/debug.h"
 #include "common/io/fdWrite.h"
 #include "common/log.h"
@@ -124,14 +125,9 @@ storageListRender(IoWrite *write)
     // Get path
     const String *path = NULL;
 
+    // Get and validate if path is valid for repo
     if (strLstSize(cfgCommandParam()) == 1)
-    {
+        path = repoPathIsValid(strLstGet(cfgCommandParam(), 0));
-        path = strLstGet(cfgCommandParam(), 0);
-
-        // Make sure the path does not end with a slash unless it is only /
-        if (strEndsWith(path, FSLASH_STR) && strSize(path) > 1)
-            path = strPath(path);
-    }
     else if (strLstSize(cfgCommandParam()) > 1)
         THROW(ParamInvalidError, "only one path may be specified");
 

src/command/repo/put.c

@@ -5,6 +5,7 @@ Repository Put Command
 
 #include <unistd.h>
 
+#include "command/repo/common.h"
 #include "common/crypto/cipherBlock.h"
 #include "common/debug.h"
 #include "common/io/fdRead.h"
@@ -34,6 +35,9 @@ storagePutProcess(IoRead *source)
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
+        // Is path valid for repo?
+        file = repoPathIsValid(file);
+
         StorageWrite *destination = storageNewWriteP(storageRepoWrite(), file);
 
         // Add encryption if needed

src/command/repo/rm.c

@@ -3,6 +3,7 @@ Repository Remove Command
 ***********************************************************************************************************************************/
 #include "build.auto.h"
 
+#include "command/repo/common.h"
 #include "common/debug.h"
 #include "common/log.h"
 #include "common/memContext.h"
@@ -25,6 +26,10 @@ cmdStorageRemove(void)
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
+        // Is path valid for repo?
+        if (path != NULL)
+            path = repoPathIsValid(path);
+
         // Check if this is a file
         StorageInfo info = storageInfoP(storageRepo(), path, .ignoreMissing = true);
 

test/code-count/file-type.yaml

@@ -447,6 +447,14 @@ src/command/remote/remote.h:
   class: core
   type: c/h
 
+src/command/repo/common.c:
+  class: core
+  type: c
+
+src/command/repo/common.h:
+  class: core
+  type: c/h
+
 src/command/repo/create.c:
   class: core
   type: c

test/define.yaml

@@ -822,6 +822,7 @@ unit:
         total: 4
 
         coverage:
+          - command/repo/common
           - command/repo/get
           - command/repo/ls
           - command/repo/put

test/src/module/command/repoTest.c

@@ -165,7 +165,18 @@ testRun(void)
         HRN_CFG_LOAD(cfgCmdRepoLs, argListTmp);
 
         TEST_ERROR(
-            storageListRender(ioBufferWriteNew(output)), AssertError, "absolute path '/' is not in base path '" TEST_PATH "/repo'");
+            storageListRender(ioBufferWriteNew(output)), ParamInvalidError,
+            "absolute path '/' is not in base path '" TEST_PATH "/repo'");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("error on //");
+
+        argListTmp = strLstDup(argList);
+        strLstAddZ(argListTmp, "bbb//");
+        HRN_CFG_LOAD(cfgCmdRepoLs, argListTmp);
+
+        TEST_ERROR(
+            storageListRender(ioBufferWriteNew(output)), ParamInvalidError, "path 'bbb//' cannot contain //");
 
         // -------------------------------------------------------------------------------------------------------------------------
         TEST_TITLE("subdirectory");
@@ -594,7 +605,7 @@ testRun(void)
 
         writeBuffer = bufNew(0);
         TEST_ERROR(
-            storageGetProcess(ioBufferWriteNew(writeBuffer)), OptionInvalidValueError,
+            storageGetProcess(ioBufferWriteNew(writeBuffer)), ParamInvalidError,
             "absolute path '/somewhere/" INFO_ARCHIVE_FILE "' is not in base path '" TEST_PATH "/repo'");
 
         // -------------------------------------------------------------------------------------------------------------------------