unzip: robustify overwrite checks

function                                             old     new   delta
get_lstat_mode                                         -      55     +55
unzip_main                                          2667    2642     -25
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/1 up/down: 55/-25)             Total: 30 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko 2017-07-20 18:56:05 +02:00
parent 13ae85edd1
commit bff9bbc73f

View File

@ -334,6 +334,7 @@ static void unzip_create_leading_dirs(const char *fn)
free(name); free(name);
} }
#if ENABLE_FEATURE_UNZIP_CDF
static void unzip_extract_symlink(zip_header_t *zip, const char *dst_fn) static void unzip_extract_symlink(zip_header_t *zip, const char *dst_fn)
{ {
char *target; char *target;
@ -365,6 +366,7 @@ static void unzip_extract_symlink(zip_header_t *zip, const char *dst_fn)
bb_perror_msg_and_die("can't create symlink '%s'", dst_fn); bb_perror_msg_and_die("can't create symlink '%s'", dst_fn);
free(target); free(target);
} }
#endif
static void unzip_extract(zip_header_t *zip, int dst_fd) static void unzip_extract(zip_header_t *zip, int dst_fd)
{ {
@ -437,6 +439,19 @@ static void my_fgets80(char *buf80)
} }
} }
static int get_lstat_mode(const char *dst_fn)
{
struct stat stat_buf;
if (lstat(dst_fn, &stat_buf) == -1) {
if (errno != ENOENT) {
bb_perror_msg_and_die("can't stat '%s'", dst_fn);
}
/* File does not exist */
return -1;
}
return stat_buf.st_mode;
}
int unzip_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int unzip_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int unzip_main(int argc, char **argv) int unzip_main(int argc, char **argv)
{ {
@ -459,7 +474,6 @@ int unzip_main(int argc, char **argv)
char *base_dir = NULL; char *base_dir = NULL;
int i, opt; int i, opt;
char key_buf[80]; /* must match size used by my_fgets80 */ char key_buf[80]; /* must match size used by my_fgets80 */
struct stat stat_buf;
/* -q, -l and -v: UnZip 5.52 of 28 February 2005, by Info-ZIP: /* -q, -l and -v: UnZip 5.52 of 28 February 2005, by Info-ZIP:
* *
@ -836,11 +850,11 @@ int unzip_main(int argc, char **argv)
goto do_extract; goto do_extract;
} }
if (last_char_is(dst_fn, '/')) { if (last_char_is(dst_fn, '/')) {
int mode;
/* Extract directory */ /* Extract directory */
if (stat(dst_fn, &stat_buf) == -1) { mode = get_lstat_mode(dst_fn);
if (errno != ENOENT) { if (mode == -1) { /* ENOENT */
bb_perror_msg_and_die("can't stat '%s'", dst_fn);
}
if (!quiet) { if (!quiet) {
printf(" creating: %s\n", dst_fn); printf(" creating: %s\n", dst_fn);
} }
@ -849,7 +863,7 @@ int unzip_main(int argc, char **argv)
xfunc_die(); xfunc_die();
} }
} else { } else {
if (!S_ISDIR(stat_buf.st_mode)) { if (!S_ISDIR(mode)) {
bb_error_msg_and_die("'%s' exists but is not a %s", bb_error_msg_and_die("'%s' exists but is not a %s",
dst_fn, "directory"); dst_fn, "directory");
} }
@ -857,30 +871,33 @@ int unzip_main(int argc, char **argv)
goto skip_cmpsize; goto skip_cmpsize;
} }
check_file: check_file:
/* Extract file */ /* Does target file already exist? */
if (lstat(dst_fn, &stat_buf) == -1) { {
/* File does not exist */ int mode = get_lstat_mode(dst_fn);
if (errno != ENOENT) { if (mode == -1) {
bb_perror_msg_and_die("can't stat '%s'", dst_fn); /* ENOENT: does not exist */
}
goto do_open_and_extract; goto do_open_and_extract;
} }
/* File already exists */
if (overwrite == O_NEVER) { if (overwrite == O_NEVER) {
goto skip_cmpsize; goto skip_cmpsize;
} }
if (!S_ISREG(stat_buf.st_mode)) { if (!S_ISREG(mode)) {
/* File is not regular file */ fishy:
bb_error_msg_and_die("'%s' exists but is not a %s", bb_error_msg_and_die("'%s' exists but is not a %s",
dst_fn, "regular file"); dst_fn, "regular file");
} }
/* File is regular file */ if (overwrite == O_ALWAYS) {
if (overwrite == O_ALWAYS)
goto do_open_and_extract; goto do_open_and_extract;
}
printf("replace %s? [y]es, [n]o, [A]ll, [N]one, [r]ename: ", dst_fn); printf("replace %s? [y]es, [n]o, [A]ll, [N]one, [r]ename: ", dst_fn);
my_fgets80(key_buf); my_fgets80(key_buf);
//TODO: redo lstat + ISREG check! user input could have taken a long time! /* User input could take a long time. Is it still a regular file? */
mode = get_lstat_mode(dst_fn);
if (!S_ISREG(mode))
goto fishy;
}
/* Extract (or skip) it */
switch (key_buf[0]) { switch (key_buf[0]) {
case 'A': case 'A':
overwrite = O_ALWAYS; overwrite = O_ALWAYS;
@ -888,10 +905,15 @@ int unzip_main(int argc, char **argv)
do_open_and_extract: do_open_and_extract:
unzip_create_leading_dirs(dst_fn); unzip_create_leading_dirs(dst_fn);
#if ENABLE_FEATURE_UNZIP_CDF #if ENABLE_FEATURE_UNZIP_CDF
if (!S_ISLNK(file_mode)) dst_fd = -1;
dst_fd = xopen3(dst_fn, O_WRONLY | O_CREAT | O_TRUNC, file_mode); if (!S_ISLNK(file_mode)) {
dst_fd = xopen3(dst_fn,
O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW,
file_mode);
}
#else #else
dst_fd = xopen(dst_fn, O_WRONLY | O_CREAT | O_TRUNC); /* O_NOFOLLOW defends against symlink attacks */
dst_fd = xopen(dst_fn, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW);
#endif #endif
do_extract: do_extract:
if (!quiet) { if (!quiet) {
@ -901,7 +923,7 @@ int unzip_main(int argc, char **argv)
} }
#if ENABLE_FEATURE_UNZIP_CDF #if ENABLE_FEATURE_UNZIP_CDF
if (S_ISLNK(file_mode)) { if (S_ISLNK(file_mode)) {
if (dst_fd != STDOUT_FILENO) /* no -p */ if (dst_fd != STDOUT_FILENO) /* not -p? */
unzip_extract_symlink(&zip, dst_fn); unzip_extract_symlink(&zip, dst_fn);
} else } else
#endif #endif