From 81dfa6b3e08f6934885ba5c98939587d6850d08e Mon Sep 17 00:00:00 2001

From: Josef Moellers <jmoellers@suse.de>

Date: Thu, 4 Oct 2018 14:21:48 +0200

Subject: [PATCH] Fix issue #62: Remove any "../" components from pathnames of

 extracted files. [CVE-2018-17828]

---

 bins/unzzipcat-big.c | 57 +++++++++++++++++++++++++++++++++++++++++++-

 bins/unzzipcat-mem.c | 57 +++++++++++++++++++++++++++++++++++++++++++-

 bins/unzzipcat-mix.c | 57 +++++++++++++++++++++++++++++++++++++++++++-

 bins/unzzipcat-zip.c | 57 +++++++++++++++++++++++++++++++++++++++++++-

 4 files changed, 224 insertions(+), 4 deletions(-)

diff --git a/bins/unzzipcat-big.c b/bins/unzzipcat-big.c

index 982d262..88c4d65 100644

--- 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 9bc966b..793bde8 100644

--- 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 91c2f00..73b6ed6 100644

--- 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 2810f85..7f7f3fa 100644

--- 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)