From cb6d041ba711620b52637ebd2f9cb0c445a91e4f Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 1 Nov 2024 14:15:09 -0700 Subject: [PATCH] Prefer other types to int in extract.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/extract.c (fd_chmod, extract_chdir, open_output_file) (extract_file, extract_link, extract_symlink, extract_node) (extract_fifo, tar_extractor_t, pepare_to_extract): Prefer char to int for typeflag, since it’s a char. All uses changed. (fd_chmod): Use clearer code for errno. (extract_dir, extract_file, create_placeholder_file, extract_link) (extract_symlink, extract_node, extract_fifo, tar_extractor_t): Return bool true for success, false for failure. All uses changed. (open_output_file): Prefer bool for boolean. (prepare_to_extract): Simplify by returning the extractor a null pointer, rather than storing through a pointer to an extractor. CVE: CVE-2026-5704 Upstream-Status: Backport [https://cgit.git.savannah.gnu.org/cgit/tar.git/commit/?id=112ead79312ea308e58414b74623f101b8c06f0b] Backport Changes: - In src/extract.c, the extractor-selection and extraction-control-flow hunk was adapted to the older tar-1.35 code layout around prepare_to_extract() and extract_archive(). The backport preserves the upstream logic but does not apply as an exact textual match in that section. (cherry picked from commit 112ead79312ea308e58414b74623f101b8c06f0b) Signed-off-by: Himanshu Jadon --- src/extract.c | 141 ++++++++++++++++++++++++-------------------------- 1 file changed, 69 insertions(+), 72 deletions(-) diff --git a/src/extract.c b/src/extract.c index 314d8bc0..b384fed3 100644 --- a/src/extract.c +++ b/src/extract.c @@ -254,9 +254,9 @@ fd_i_chmod (int fd, char const *file, mode_t mode, int atflag) notation. */ static int -fd_chmod(int fd, char const *file_name, int mode, int atflag, int typeflag) +fd_chmod (int fd, char const *file_name, int mode, int atflag, char typeflag) { - int chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno; + int chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) < 0 ? errno : 0; /* On Solaris, chmod may fail if we don't have PRIV_ALL, because setuid-root files would otherwise be a backdoor. See @@ -265,7 +265,7 @@ fd_chmod(int fd, char const *file_name, int mode, int atflag, int typeflag) if (chmod_errno == EPERM && (mode & S_ISUID) && priv_set_restore_linkdir () == 0) { - chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno; + chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) < 0 ? errno : 0; priv_set_remove_linkdir (); } @@ -275,7 +275,7 @@ fd_chmod(int fd, char const *file_name, int mode, int atflag, int typeflag) supported and if the file is not a symlink. This introduces a race, alas. */ if (atflag && typeflag != SYMTYPE && ! implemented (chmod_errno)) - chmod_errno = fd_i_chmod (fd, file_name, mode, 0) == 0 ? 0 : errno; + chmod_errno = fd_i_chmod (fd, file_name, mode, 0) < 0 ? errno : 0; if (chmod_errno && (typeflag != SYMTYPE || implemented (chmod_errno))) { @@ -1036,8 +1036,8 @@ safe_dir_mode (struct stat const *st) /* Extractor functions for various member types */ -static int -extract_dir (char *file_name, int typeflag) +static bool +extract_dir (char *file_name, char typeflag) { int status; mode_t mode; @@ -1089,7 +1089,7 @@ extract_dir (char *file_name, int typeflag) if (keep_directory_symlink_option && is_directory_link (file_name, &st)) - return 0; + return true; if ((st.st_mode != 0 && fstatat_flags == 0) || deref_stat (file_name, &st) == 0) @@ -1102,7 +1102,7 @@ extract_dir (char *file_name, int typeflag) if (interdir_made) { repair_delayed_set_stat (file_name, &st); - return 0; + return true; } else if (old_files_option == NO_OVERWRITE_DIR_OLD_FILES) { @@ -1154,7 +1154,7 @@ extract_dir (char *file_name, int typeflag) if (errno != EEXIST) { mkdir_error (file_name); - return 1; + return false; } break; } @@ -1167,13 +1167,13 @@ extract_dir (char *file_name, int typeflag) delay_set_stat (file_name, ¤t_stat_info, current_mode, current_mode_mask, current_stat_info.stat.st_mode, atflag); - return status; + return status == 0; } static int -open_output_file (char const *file_name, int typeflag, mode_t mode, +open_output_file (char const *file_name, char typeflag, mode_t mode, int file_created, mode_t *current_mode, mode_t *current_mode_mask) { @@ -1189,11 +1189,11 @@ open_output_file (char const *file_name, int typeflag, mode_t mode, if (typeflag == CONTTYPE) { - static int conttype_diagnosed; + static bool conttype_diagnosed; if (!conttype_diagnosed) { - conttype_diagnosed = 1; + conttype_diagnosed = true; WARNOPT (WARN_CONTIGUOUS_CAST, (0, 0, _("Extracting contiguous files as regular files"))); } @@ -1245,8 +1245,8 @@ open_output_file (char const *file_name, int typeflag, mode_t mode, return fd; } -static int -extract_file (char *file_name, int typeflag) +static bool +extract_file (char *file_name, char typeflag) { int fd; off_t size; @@ -1268,7 +1268,7 @@ extract_file (char *file_name, int typeflag) if (fd < 0) { skip_member (); - return 0; + return true; } } else @@ -1291,9 +1291,9 @@ extract_file (char *file_name, int typeflag) { skip_member (); if (recover == RECOVER_SKIP) - return 0; + return true; open_error (file_name); - return 1; + return false; } } } @@ -1344,7 +1344,7 @@ extract_file (char *file_name, int typeflag) it doesn't exist, or we don't want to touch it anyway. */ if (to_stdout_option) - return 0; + return true; if (! to_command_option) set_stat (file_name, ¤t_stat_info, fd, @@ -1359,7 +1359,7 @@ extract_file (char *file_name, int typeflag) if (to_command_option) sys_wait_command (); - return status; + return status == 0; } /* Return true if NAME is a delayed link. This can happen only if the link @@ -1399,7 +1399,7 @@ find_delayed_link_source (char const *name) process. */ -static int +static bool create_placeholder_file (char *file_name, bool is_symlink, bool *interdir_made) { int fd; @@ -1413,7 +1413,7 @@ create_placeholder_file (char *file_name, bool is_symlink, bool *interdir_made) that the link being extracted is a duplicate of an already processed one. Skip it. */ - return 0; + return true; } switch (maybe_recoverable (file_name, false, interdir_made)) @@ -1422,11 +1422,11 @@ create_placeholder_file (char *file_name, bool is_symlink, bool *interdir_made) continue; case RECOVER_SKIP: - return 0; + return true; case RECOVER_NO: open_error (file_name); - return -1; + return false; } } @@ -1484,14 +1484,14 @@ create_placeholder_file (char *file_name, bool is_symlink, bool *interdir_made) if ((h = find_direct_ancestor (file_name)) != NULL) mark_after_links (h); - return 0; + return true; } - return -1; + return false; } -static int -extract_link (char *file_name, MAYBE_UNUSED int typeflag) +static bool +extract_link (char *file_name, MAYBE_UNUSED char typeflag) { bool interdir_made = false; char const *link_name; @@ -1531,7 +1531,7 @@ extract_link (char *file_name, MAYBE_UNUSED int typeflag) } } - return 0; + return true; } else if ((e == EEXIST && strcmp (link_name, file_name) == 0) || ((fstatat (chdir_fd, link_name, &st1, AT_SYMLINK_NOFOLLOW) @@ -1540,7 +1540,7 @@ extract_link (char *file_name, MAYBE_UNUSED int typeflag) == 0) && st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino)) - return 0; + return true; errno = e; } @@ -1548,17 +1548,17 @@ extract_link (char *file_name, MAYBE_UNUSED int typeflag) == RECOVER_OK); if (rc == RECOVER_SKIP) - return 0; + return true; if (!(incremental_option && errno == EEXIST)) { link_error (link_name, file_name); - return 1; + return false; } - return 0; + return true; } -static int -extract_symlink (char *file_name, MAYBE_UNUSED int typeflag) +static bool +extract_symlink (char *file_name, MAYBE_UNUSED char typeflag) { #ifdef HAVE_SYMLINK bool interdir_made = false; @@ -1575,16 +1575,16 @@ extract_symlink (char *file_name, MAYBE_UNUSED int typeflag) continue; case RECOVER_SKIP: - return 0; + return true; case RECOVER_NO: symlink_error (current_stat_info.link_name, file_name); - return -1; + return false; } set_stat (file_name, ¤t_stat_info, -1, 0, 0, SYMTYPE, false, AT_SYMLINK_NOFOLLOW); - return 0; + return true; #else static int warned_once; @@ -1601,8 +1601,8 @@ extract_symlink (char *file_name, MAYBE_UNUSED int typeflag) } #if S_IFCHR || S_IFBLK -static int -extract_node (char *file_name, int typeflag) +static bool +extract_node (char *file_name, char typeflag) { bool interdir_made = false; mode_t mode = (current_stat_info.stat.st_mode & (MODE_RWX | S_IFBLK | S_IFCHR) @@ -1616,23 +1616,23 @@ extract_node (char *file_name, int typeflag) continue; case RECOVER_SKIP: - return 0; + return true; case RECOVER_NO: mknod_error (file_name); - return -1; + return false; } set_stat (file_name, ¤t_stat_info, -1, mode & ~ current_umask, MODE_RWX, typeflag, false, AT_SYMLINK_NOFOLLOW); - return 0; + return true; } #endif #if HAVE_MKFIFO || defined mkfifo -static int -extract_fifo (char *file_name, int typeflag) +static bool +extract_fifo (char *file_name, char typeflag) { bool interdir_made = false; mode_t mode = (current_stat_info.stat.st_mode & MODE_RWX @@ -1645,31 +1645,31 @@ extract_fifo (char *file_name, int typeflag) continue; case RECOVER_SKIP: - return 0; + return true; case RECOVER_NO: mkfifo_error (file_name); - return -1; + return false; } set_stat (file_name, ¤t_stat_info, -1, mode & ~ current_umask, MODE_RWX, typeflag, false, AT_SYMLINK_NOFOLLOW); - return 0; + return true; } #endif -typedef int (*tar_extractor_t) (char *file_name, int typeflag); +typedef bool (*tar_extractor_t) (char *file_name, char typeflag); /* Prepare to extract a file. Find extractor function. - Return true to proceed with the extraction, false to skip the current - member. */ + Return an extractor to proceed with the extraction, + a null pointer to skip the current member. */ -static bool -prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) +static tar_extractor_t +prepare_to_extract (char const *file_name, char typeflag) { - tar_extractor_t extractor = NULL; + tar_extractor_t extractor; /* Select the extractor */ switch (typeflag) @@ -1683,10 +1683,8 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) case CONTTYPE: /* Appears to be a file. But BSD tar uses the convention that a slash suffix means a directory. */ - if (current_stat_info.had_trailing_slash) - extractor = extract_dir; - else - extractor = extract_file; + extractor = (current_stat_info.had_trailing_slash + ? extract_dir : extract_file); break; case SYMTYPE: @@ -1725,18 +1723,18 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) break; case GNUTYPE_VOLHDR: - return false; + return NULL; case GNUTYPE_MULTIVOL: ERROR ((0, 0, _("%s: Cannot extract -- file is continued from another volume"), quotearg_colon (current_stat_info.file_name))); - return false; + return NULL; case GNUTYPE_LONGNAME: case GNUTYPE_LONGLINK: ERROR ((0, 0, _("Unexpected long name header"))); - return false; + return NULL; default: WARNOPT (WARN_UNKNOWN_CAST, @@ -1749,7 +1747,7 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) if (EXTRACT_OVER_PIPE) { if (extractor != extract_file) - return false; + return NULL; } else { @@ -1763,7 +1761,7 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) && errno && errno != ENOENT) { unlink_error (file_name); - return false; + return NULL; } break; @@ -1773,7 +1771,7 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) WARNOPT (WARN_IGNORE_NEWER, (0, 0, _("Current %s is newer or same age"), quote (file_name))); - return false; + return NULL; } break; @@ -1781,9 +1779,7 @@ prepare_to_extract (char const *file_name, int typeflag, tar_extractor_t *fun) break; } } - *fun = extractor; - - return true; + return extractor; } /* Extract a file from the archive. */ @@ -1791,7 +1787,6 @@ void extract_archive (void) { char typeflag; - tar_extractor_t fun; bool skip_dotdot_name; fatal_exit_hook = extract_finish; @@ -1841,12 +1836,14 @@ extract_archive (void) /* Extract the archive entry according to its type. */ /* KLUDGE */ - typeflag = sparse_member_p (¤t_stat_info) ? - GNUTYPE_SPARSE : current_header->header.typeflag; + typeflag = (sparse_member_p (¤t_stat_info) + ? GNUTYPE_SPARSE : current_header->header.typeflag); - if (prepare_to_extract (current_stat_info.file_name, typeflag, &fun)) + tar_extractor_t fun = prepare_to_extract (current_stat_info.file_name, + typeflag); + if (fun) { - if (fun (current_stat_info.file_name, typeflag) == 0) + if (fun (current_stat_info.file_name, typeflag)) return; } else -- 2.44.1