Fix: CVE-2018-17828

Resolves: CVE-2018-17828
Related: #1635890
This commit is contained in:
Jakub Martisko 2019-01-24 13:52:33 +01:00
parent a45bdf8fab
commit d2c611006e
2 changed files with 355 additions and 1 deletions

341
CVE-2018-17828.patch Normal file
View File

@ -0,0 +1,341 @@
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)

View File

@ -1,12 +1,17 @@
Summary: Lightweight library to easily extract data from zip files
Name: zziplib
Version: 0.13.69
Release: 1%{?dist}
Release: 2%{?dist}
License: LGPLv2+ or MPLv1.1
Group: Applications/Archiving
URL: http://zziplib.sourceforge.net/
Source: https://github.com/gdraheim/zziplib/archive/v%{version}.tar.gz
Patch0: zziplib-0.13.69-multilib.patch
Patch1: CVE-2018-17828.patch
Patch2: CVE-2018-17828.part2.patch
BuildRequires: gcc
BuildRequires: perl-interpreter
BuildRequires: python2
@ -60,6 +65,10 @@ zziplib library.
%prep
%setup -q
%patch1 -p1
%patch2 -p1
# Force py2 for the build
find . -name '*.py' | xargs sed -i 's@#! /usr/bin/python@#! %__python2@g;s@#! /usr/bin/env python@#! %__python2@g'
@ -107,6 +116,10 @@ make install DESTDIR=%{buildroot}
%{_mandir}/man3/*
%changelog
* Thu Jan 24 2019 Jakub Martisko <jamartis@redhat.com> - 0.13.69-2
Related: 1635890
Resolves: CVE-2018-17828
* Mon Jul 23 2018 Alexander Bokovoy <abokovoy@redhat.com> - 0.13.69-1
- Update to 0.13.69 release
- Fixes: #1598246 (CVE-2018-6541)