Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions src/zip.c
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,27 @@ static mz_bool zip_symlink_target_escapes(const char *link_name,
}
#endif

// Creates the directory of a directory entry whose name carries no trailing
// separator. zip_mkpath only acts on components that end at a separator, so
// such an entry never reaches it. Refuses a path already taken by something
// that is not a directory, the way zip_mkpath does for the trailing-separator
// spelling of the same entry.
static int zip_mkdir_entry(const char *path) {
struct MZ_FILE_STAT_STRUCT st;

if (MZ_MKDIR(path) == 0) {
return 0;
}
if (errno != EEXIST) {
return ZIP_EMKDIR;
}
if (MZ_FILE_STAT(path, &st) < 0) {
return ZIP_ECHKDIR;
}

return ((st.st_mode & S_IFMT) == S_IFDIR) ? 0 : ZIP_ECHKDIR;
}

static int zip_archive_extract(mz_zip_archive *zip_archive, const char *dir,
int (*on_extract)(const char *filename,
void *arg),
Expand All @@ -631,7 +652,7 @@ static int zip_archive_extract(mz_zip_archive *zip_archive, const char *dir,
char symlink_to[MZ_ZIP_MAX_ARCHIVE_FILENAME_SIZE + 1];
#endif
mz_zip_archive_file_stat info;
size_t dirlen = 0, filename_size;
size_t dirlen = 0, filename_size, namelen = 0;
mz_uint32 xattr = 0;

memset(path, 0, sizeof(path));
Expand Down Expand Up @@ -678,12 +699,14 @@ static int zip_archive_extract(mz_zip_archive *zip_archive, const char *dir,
goto out;
}

namelen = strlen(info.m_filename);

// a name built only from separators and "."/".." components (e.g. "..",
// "/", "./") normalizes to an empty string, so the path below collapses to
// the destination directory itself: a directory-flagged entry would then
// CHMOD the destination to the archive's mode and a regular entry would try
// to write over it
if (info.m_filename[0] == '\0') {
if (namelen == 0) {
err = ZIP_EINVENTNAME;
goto out;
}
Expand All @@ -692,7 +715,7 @@ static int zip_archive_extract(mz_zip_archive *zip_archive, const char *dir,
// symlink branch then measures escape depth from the full name while the
// link is created at the shortened path, so a long name plus a climbing
// target resolves outside the destination
if (strlen(info.m_filename) >= filename_size) {
if (namelen >= filename_size) {
err = ZIP_EINVENTNAME;
goto out;
}
Expand Down Expand Up @@ -765,6 +788,15 @@ static int zip_archive_extract(mz_zip_archive *zip_archive, const char *dir,
err = ZIP_ENOFILE;
goto out;
}
} else if (!ISSLASH(info.m_filename[namelen - 1])) {
// a directory entry can also be spelled with the DOS directory
// attribute and no trailing separator. nothing creates that directory,
// so the mode restored below used to land on whatever already occupied
// the path: an archive could relax the permissions of a file in the
// destination it never wrote, and extraction still reported success
if ((err = zip_mkdir_entry(path)) < 0) {
goto out;
}
}

#if defined(_MSC_VER) || defined(PS4)
Expand Down
72 changes: 72 additions & 0 deletions test/test_extract.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,76 @@ MU_TEST(test_extract_dotdot_name_chmod_rejected) {
mu_check(system(rm) == 0);
}

MU_TEST(test_extract_dirflag_no_slash_chmod_rejected) {
// a directory entry can also be spelled with the DOS directory attribute and
// no trailing separator. nothing creates that directory, so the mode restore
// fell through onto whatever already occupied the path: "keep.txt" went from
// 0600 to 0777 without a byte of it being written, and zip_extract returned 0
static const struct sym_entry_t entries[] = {
{"keep.txt", "", 0, 1, 0x01FF0000UL}, // dir flag + unix mode 0777
};
char tmpl[] = "ext-XXXXXX";
char *dir = mkdtemp(tmpl);
char victim[512];
char content[32];
char rm[512];
struct stat st;
FILE *fp;
size_t nread;
mu_check(dir != NULL);

// a file already in the destination that the archive never writes
snprintf(victim, sizeof(victim), "%s/keep.txt", dir);
fp = fopen(victim, "wb");
mu_check(fp != NULL);
mu_check(fwrite("SECRET", 1, 6, fp) == 6);
fclose(fp);
mu_check(chmod(victim, 0600) == 0);

sym_write_zip(ZIPNAME, entries, 1);
mu_assert_int_eq(ZIP_ECHKDIR, zip_extract(ZIPNAME, dir, NULL, NULL));

// it keeps both its bytes and its permissions
mu_assert_int_eq(0, stat(victim, &st));
mu_assert_int_eq(0600, (int)(st.st_mode & 0777));
fp = fopen(victim, "rb");
mu_check(fp != NULL);
memset(content, 0, sizeof(content));
nread = fread(content, 1, sizeof(content) - 1, fp);
fclose(fp);
mu_assert_int_eq(6, (int)nread);
mu_assert_int_eq(0, strncmp(content, "SECRET", 6));

snprintf(rm, sizeof(rm), "rm -rf %s", dir);
mu_check(system(rm) == 0);
}

MU_TEST(test_extract_dirflag_no_slash_created) {
// the same entry on a free path: the directory is created and carries the
// archived mode, where before nothing was created and the mode restore failed
// on the missing path, aborting the whole extraction with ZIP_ENOPERM
static const struct sym_entry_t entries[] = {
{"newdir", "", 0, 1, 0x01ED0000UL}, // dir flag + unix mode 0755
};
char tmpl[] = "ext-XXXXXX";
char *dir = mkdtemp(tmpl);
char created[512];
char rm[512];
struct stat st;
mu_check(dir != NULL);

sym_write_zip(ZIPNAME, entries, 1);
mu_assert_int_eq(0, zip_extract(ZIPNAME, dir, NULL, NULL));

snprintf(created, sizeof(created), "%s/newdir", dir);
mu_assert_int_eq(0, stat(created, &st));
mu_check(S_ISDIR(st.st_mode));
mu_assert_int_eq(0755, (int)(st.st_mode & 0777));

snprintf(rm, sizeof(rm), "rm -rf %s", dir);
mu_check(system(rm) == 0);
}

MU_TEST(test_extract_symlink_writethrough_rejected) {
// a regular entry whose name matches a symlink stored earlier in the same
// archive is written with fopen("wb"), which would follow that link and write
Expand Down Expand Up @@ -637,6 +707,8 @@ MU_TEST_SUITE(test_extract_suite) {
MU_RUN_TEST(test_extract_symlink_longname_rejected);
MU_RUN_TEST(test_extract_chardev_not_symlink);
MU_RUN_TEST(test_extract_dotdot_name_chmod_rejected);
MU_RUN_TEST(test_extract_dirflag_no_slash_chmod_rejected);
MU_RUN_TEST(test_extract_dirflag_no_slash_created);
MU_RUN_TEST(test_extract_symlink_writethrough_rejected);
#endif
}
Expand Down
Loading