gdraheim / zziplib

The ZZIPlib provides read access on ZIP-archives and unpacked data. It features an additional simplified API following the standard Posix API for file access
Other
64 stars 53 forks source link

Directory traversal vulnerability in zziplib 0.13.69 #62

Open 92wyunchao opened 6 years ago

92wyunchao commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

https://github.com/gdraheim/zziplib/pull/63

jamartis commented 6 years ago

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

0-wiz-0 commented 4 days ago

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?