Fix issue #62: Remove any "../" components from pathnames of extracted files.

[CVE-2018-17828]

diff --git a/bins/unzzipcat-big.c b/bins/unzzipcat-big.c
index 982d262dbe83b1a39ed5552451e041c5cc574596..88c4d658bad9d8b1d48fba207e18abb98557e93b 100644 (file)
--- a/bins/unzzipcat-big.c
+++ b/bins/unzzipcat-big.c
@@ -53,6 +53,48 @@ static void unzzip_cat_file(FILE* disk, char* name, FILE* out)
     }
 }
 
+/*
+ * NAME: remove_dotdotslash
+ * PURPOSE: To remove any "../" components from the given pathname
+ * ARGUMENTS: path: path name with maybe "../" components
+ * RETURNS: Nothing, "path" is modified in-place
+ * NOTE: removing "../" from the path ALWAYS shortens the path, never adds to it!
+ *     Also, "path" is not used after creating it.
+ *     So modifying "path" in-place is safe to do.
+ */
+static inline void
+remove_dotdotslash(char *path)
+{
+    /* Note: removing "../" from the path ALWAYS shortens the path, never adds to it! */
+    char *dotdotslash;
+    int warned = 0;
+
+    dotdotslash = path;
+    while ((dotdotslash = strstr(dotdotslash, "../")) != NULL)
+    {
+        /*
+         * Remove only if at the beginning of the pathname ("../path/name")
+         * or when preceded by a slash ("path/../name"),
+         * otherwise not ("path../name..")!
+         */
+        if (dotdotslash == path || dotdotslash[-1] == '/')
+        {
+            char *src, *dst;
+            if (!warned)
+            {
+                /* Note: the first time through the pathname is still intact */
+                fprintf(stderr, "Removing \"../\" path component(s) in %s\n", path);
+                warned = 1;
+            }
+            /* We cannot use strcpy(), as there "The strings may not overlap" */
+            for (src = dotdotslash+3, dst=dotdotslash; (*dst = *src) != '\0'; src++, dst++)
+                ;
+        }
+        else
+            dotdotslash +=3;   /* skip this instance to prevent infinite loop */
+    }
+}
+
 static void makedirs(const char* name)
 {
       char* p = strrchr(name, '/');
@@ -70,6 +112,16 @@ static void makedirs(const char* name)
 
 static FILE* create_fopen(char* name, char* mode, int subdirs)
 {
+   char *name_stripped;
+   FILE *fp;
+   int mustfree = 0;
+
+   if ((name_stripped = strdup(name)) != NULL)
+   {
+       remove_dotdotslash(name_stripped);
+       name = name_stripped;
+       mustfree = 1;
+   }
    if (subdirs)
    {
       char* p = strrchr(name, '/');
@@ -79,7 +131,10 @@ static FILE* create_fopen(char* name, char* mode, int subdirs)
           free (dir_name);
       }
    }
-   return fopen(name, mode);      
+   fp = fopen(name, mode);
+   if (mustfree)
+       free(name_stripped);
+    return fp;
 }
 
 

diff --git a/bins/unzzipcat-mem.c b/bins/unzzipcat-mem.c
index 9bc966b90db5000603e12ebf00a4b278a66d2815..793bde85ce8b5bc9f6012930e81a00dab4bbea7c 100644 (file)
--- a/bins/unzzipcat-mem.c
+++ b/bins/unzzipcat-mem.c
@@ -58,6 +58,48 @@ static void unzzip_mem_disk_cat_file(ZZIP_MEM_DISK* disk, char* name, FILE* out)
     }
 }
 
+/*
+ * NAME: remove_dotdotslash
+ * PURPOSE: To remove any "../" components from the given pathname
+ * ARGUMENTS: path: path name with maybe "../" components
+ * RETURNS: Nothing, "path" is modified in-place
+ * NOTE: removing "../" from the path ALWAYS shortens the path, never adds to it!
+ *     Also, "path" is not used after creating it.
+ *     So modifying "path" in-place is safe to do.
+ */
+static inline void
+remove_dotdotslash(char *path)
+{
+    /* Note: removing "../" from the path ALWAYS shortens the path, never adds to it! */
+    char *dotdotslash;
+    int warned = 0;
+
+    dotdotslash = path;
+    while ((dotdotslash = strstr(dotdotslash, "../")) != NULL)
+    {
+        /*
+         * Remove only if at the beginning of the pathname ("../path/name")
+         * or when preceded by a slash ("path/../name"),
+         * otherwise not ("path../name..")!
+         */
+        if (dotdotslash == path || dotdotslash[-1] == '/')
+        {
+            char *src, *dst;
+            if (!warned)
+            {
+                /* Note: the first time through the pathname is still intact */
+                fprintf(stderr, "Removing \"../\" path component(s) in %s\n", path);
+                warned = 1;
+            }
+            /* We cannot use strcpy(), as there "The strings may not overlap" */
+            for (src = dotdotslash+3, dst=dotdotslash; (*dst = *src) != '\0'; src++, dst++)
+                ;
+        }
+        else
+            dotdotslash +=3;   /* skip this instance to prevent infinite loop */
+    }
+}
+
 static void makedirs(const char* name)
 {
       char* p = strrchr(name, '/');
@@ -75,6 +117,16 @@ static void makedirs(const char* name)
 
 static FILE* create_fopen(char* name, char* mode, int subdirs)
 {
+   char *name_stripped;
+   FILE *fp;
+   int mustfree = 0;
+
+   if ((name_stripped = strdup(name)) != NULL)
+   {
+       remove_dotdotslash(name_stripped);
+       name = name_stripped;
+       mustfree = 1;
+   }
    if (subdirs)
    {
       char* p = strrchr(name, '/');
@@ -84,7 +136,10 @@ static FILE* create_fopen(char* name, char* mode, int subdirs)
           free (dir_name);
       }
    }
-   return fopen(name, mode);      
+   fp = fopen(name, mode);
+   if (mustfree)
+       free(name_stripped);
+    return fp;
 }
 
 static int unzzip_cat (int argc, char ** argv, int extract)

diff --git a/bins/unzzipcat-mix.c b/bins/unzzipcat-mix.c
index 91c2f009c0fc9fa29487d9e910199fcfb3994698..73b6ed667a2955834d58437ab5ae837064c28e91 100644 (file)
--- a/bins/unzzipcat-mix.c
+++ b/bins/unzzipcat-mix.c
@@ -69,6 +69,48 @@ static void unzzip_cat_file(ZZIP_DIR* disk, char* name, FILE* out)
     }
 }
 
+/*
+ * NAME: remove_dotdotslash
+ * PURPOSE: To remove any "../" components from the given pathname
+ * ARGUMENTS: path: path name with maybe "../" components
+ * RETURNS: Nothing, "path" is modified in-place
+ * NOTE: removing "../" from the path ALWAYS shortens the path, never adds to it!
+ *     Also, "path" is not used after creating it.
+ *     So modifying "path" in-place is safe to do.
+ */
+static inline void
+remove_dotdotslash(char *path)
+{
+    /* Note: removing "../" from the path ALWAYS shortens the path, never adds to it! */
+    char *dotdotslash;
+    int warned = 0;
+
+    dotdotslash = path;
+    while ((dotdotslash = strstr(dotdotslash, "../")) != NULL)
+    {
+        /*
+         * Remove only if at the beginning of the pathname ("../path/name")
+         * or when preceded by a slash ("path/../name"),
+         * otherwise not ("path../name..")!
+         */
+        if (dotdotslash == path || dotdotslash[-1] == '/')
+        {
+            char *src, *dst;
+            if (!warned)
+            {
+                /* Note: the first time through the pathname is still intact */
+                fprintf(stderr, "Removing \"../\" path component(s) in %s\n", path);
+                warned = 1;
+            }
+            /* We cannot use strcpy(), as there "The strings may not overlap" */
+            for (src = dotdotslash+3, dst=dotdotslash; (*dst = *src) != '\0'; src++, dst++)
+                ;
+        }
+        else
+            dotdotslash +=3;   /* skip this instance to prevent infinite loop */
+    }
+}
+
 static void makedirs(const char* name)
 {
       char* p = strrchr(name, '/');
@@ -86,6 +128,16 @@ static void makedirs(const char* name)
 
 static FILE* create_fopen(char* name, char* mode, int subdirs)
 {
+   char *name_stripped;
+   FILE *fp;
+   int mustfree = 0;
+
+   if ((name_stripped = strdup(name)) != NULL)
+   {
+       remove_dotdotslash(name_stripped);
+       name = name_stripped;
+       mustfree = 1;
+   }
    if (subdirs)
    {
       char* p = strrchr(name, '/');
@@ -95,7 +147,10 @@ static FILE* create_fopen(char* name, char* mode, int subdirs)
           free (dir_name);
       }
    }
-   return fopen(name, mode);      
+   fp = fopen(name, mode);
+   if (mustfree)
+       free(name_stripped);
+    return fp;
 }
 
 static int unzzip_cat (int argc, char ** argv, int extract)

diff --git a/bins/unzzipcat-zip.c b/bins/unzzipcat-zip.c
index 2810f85a42ba948f4c157210b648b9626e244e86..7f7f3faa3e1fb9216d2cad18a0b2a4992a989410 100644 (file)
--- a/bins/unzzipcat-zip.c
+++ b/bins/unzzipcat-zip.c
@@ -69,6 +69,48 @@ static void unzzip_cat_file(ZZIP_DIR* disk, char* name, FILE* out)
     }
 }
 
+/*
+ * NAME: remove_dotdotslash
+ * PURPOSE: To remove any "../" components from the given pathname
+ * ARGUMENTS: path: path name with maybe "../" components
+ * RETURNS: Nothing, "path" is modified in-place
+ * NOTE: removing "../" from the path ALWAYS shortens the path, never adds to it!
+ *     Also, "path" is not used after creating it.
+ *     So modifying "path" in-place is safe to do.
+ */
+static inline void
+remove_dotdotslash(char *path)
+{
+    /* Note: removing "../" from the path ALWAYS shortens the path, never adds to it! */
+    char *dotdotslash;
+    int warned = 0;
+
+    dotdotslash = path;
+    while ((dotdotslash = strstr(dotdotslash, "../")) != NULL)
+    {
+        /*
+         * Remove only if at the beginning of the pathname ("../path/name")
+         * or when preceded by a slash ("path/../name"),
+         * otherwise not ("path../name..")!
+         */
+        if (dotdotslash == path || dotdotslash[-1] == '/')
+        {
+            char *src, *dst;
+            if (!warned)
+            {
+                /* Note: the first time through the pathname is still intact */
+                fprintf(stderr, "Removing \"../\" path component(s) in %s\n", path);
+                warned = 1;
+            }
+            /* We cannot use strcpy(), as there "The strings may not overlap" */
+            for (src = dotdotslash+3, dst=dotdotslash; (*dst = *src) != '\0'; src++, dst++)
+                ;
+        }
+        else
+            dotdotslash +=3;   /* skip this instance to prevent infinite loop */
+    }
+}
+
 static void makedirs(const char* name)
 {
       char* p = strrchr(name, '/');
@@ -86,6 +128,16 @@ static void makedirs(const char* name)
 
 static FILE* create_fopen(char* name, char* mode, int subdirs)
 {
+   char *name_stripped;
+   FILE *fp;
+   int mustfree = 0;
+
+   if ((name_stripped = strdup(name)) != NULL)
+   {
+       remove_dotdotslash(name_stripped);
+       name = name_stripped;
+       mustfree = 1;
+   }
    if (subdirs)
    {
       char* p = strrchr(name, '/');
@@ -95,7 +147,10 @@ static FILE* create_fopen(char* name, char* mode, int subdirs)
           free (dir_name);
       }
    }
-   return fopen(name, mode);
+   fp = fopen(name, mode);
+   if (mustfree)
+       free(name_stripped);
+    return fp;
 }
 
 static int unzzip_cat (int argc, char ** argv, int extract)