Directory traversal vulnerability in zziplib 0.13.69

[92wyunchao]

Directory traversal vulnerability in zziplib 0.13.69 allows attackers to overwrite arbitrary files via a .. (dot dot) in an zip file.
$unzzip-mem evil.zip
evil.zip

Relevant code in function unzzip_cat in Unzzipcat-mem.c:
static int unzzip_cat (int argc, char ** argv, int extract)
{
......
if (argc == 2)
{ /* print directory list /
ZZIP_MEM_ENTRY entry = zzip_mem_disk_findfirst(disk);
DBG2("findfirst %p\n", entry);
for (; entry ; entry = zzip_mem_disk_findnext(disk, entry))
{
char name = zzip_mem_entry_to_name (entry);*
FILE* out = stdout;
if (extract) out = create_fopen(name, "wb", 1); //no checkout here
if (! out) {
if (errno != EISDIR) {
DBG3("can not open output file %i %s", errno, strerror(errno));
done = EXIT_ERRORS;
}
continue;
}
unzzip_mem_disk_cat_file (disk, name, out);
if (extract) fclose(out);
}
}
......
}

static FILE* create_fopen(char* name, char* mode, int subdirs)
{
if (subdirs)
{
char* p = strrchr(name, '/');
if (p) {
char* dir_name = _zzip_strndup(name, p-name);
makedirs(dir_name);
free (dir_name);
}
}
return fopen(name, mode);
}

static void unzzip_mem_disk_cat_file(ZZIP_MEM_DISK* disk, char* name, FILE* out)
{
ZZIP_DISK_FILE* file = zzip_mem_disk_fopen (disk, name);
if (file)
{
char buffer[1025]; int len;
while ((len = zzip_mem_disk_fread (buffer, 1, 1024, file)))
{
fwrite (buffer, 1, len, out);
}
zzip_mem_disk_fclose (file);
}
}

[jmoellers]

Other "zip/unzip" programs just delete any "../" components from the path. Should zziplib do the same?
Added note: I'm not sure about this, as this would then unintendedly over-write yet another file!
Simply drop any pathname containing "../" or add an option to allow "../" in extracted pathnames?
NB This issue also exists in the other unzzipcat-*.c files!

[jmoellers]

Proposed solution:
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 */
}

}
then insert "remove_dotdotslash(dir_name);" before "makedirs(dir_name)".

[jmoellers]

#63

[jamartis]

Hey, unzip-mem (single z) seems to have the same problem.
patch.txt

[0-wiz-0]

https://nvd.nist.gov/vuln/detail/CVE-2018-17828 points to this ticket, and it seems to have open questions. Can you please fix this?