From 90881d24d3f6d5fb207e97df3b91bbea8598e84e Mon Sep 17 00:00:00 2001 From: Martin Matuska Date: Tue, 29 Nov 2016 16:47:37 +0100 Subject: [PATCH 1/2] archive_write_disk_posix.c: make *_fsobj functions more readable Upstream-Status: Backported Signed-off-by: Amarnath Valluri --- libarchive/archive_write_disk_posix.c | 121 +++++++++++++++++----------------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c index 17c23b0..d786bc2 100644 --- a/libarchive/archive_write_disk_posix.c +++ b/libarchive/archive_write_disk_posix.c @@ -336,6 +336,8 @@ struct archive_write_disk { #define HFS_BLOCKS(s) ((s) >> 12) +static void fsobj_error(int *, struct archive_string *, int, const char *, + const char *); static int check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags); static int check_symlinks(struct archive_write_disk *); static int create_filesystem_object(struct archive_write_disk *); @@ -2005,8 +2007,9 @@ restore_entry(struct archive_write_disk *a) if (en) { /* Everything failed; give up here. */ - archive_set_error(&a->archive, en, "Can't create '%s'", - a->name); + if ((&a->archive)->error == NULL) + archive_set_error(&a->archive, en, "Can't create '%s'", + a->name); return (ARCHIVE_FAILED); } @@ -2388,6 +2391,17 @@ current_fixup(struct archive_write_disk *a, const char *pathname) return (a->current_fixup); } +/* Error helper for new *_fsobj functions */ +static void +fsobj_error(int *a_eno, struct archive_string *a_estr, + int err, const char *errstr, const char *path) +{ + if (a_eno) + *a_eno = err; + if (a_estr) + archive_string_sprintf(a_estr, errstr, path); +} + /* * TODO: Someday, integrate this with the deep dir support; they both * scan the path and both can be optimized by comparing against other @@ -2400,7 +2414,7 @@ current_fixup(struct archive_write_disk *a, const char *pathname) * ARCHIVE_OK if there are none, otherwise puts an error in errmsg. */ static int -check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags) +check_symlinks_fsobj(char *path, int *a_eno, struct archive_string *a_estr, int flags) { #if !defined(HAVE_LSTAT) /* Platform doesn't have lstat, so we can't look for symlinks. */ @@ -2474,19 +2488,20 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error if (errno == ENOENT) { break; } else { - /* Treat any other error as fatal - best to be paranoid here - * Note: This effectively disables deep directory - * support when security checks are enabled. - * Otherwise, very long pathnames that trigger - * an error here could evade the sandbox. - * TODO: We could do better, but it would probably - * require merging the symlink checks with the - * deep-directory editing. */ - if (error_number) *error_number = errno; - if (error_string) - archive_string_sprintf(error_string, - "Could not stat %s", - path); + /* + * Treat any other error as fatal - best to be + * paranoid here. + * Note: This effectively disables deep + * directory support when security checks are + * enabled. Otherwise, very long pathnames that + * trigger an error here could evade the + * sandbox. + * TODO: We could do better, but it would + * probably require merging the symlink checks + * with the deep-directory editing. + */ + fsobj_error(a_eno, a_estr, errno, + "Could not stat %s", path); res = ARCHIVE_FAILED; break; } @@ -2494,11 +2509,8 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error if (!last) { if (chdir(head) != 0) { tail[0] = c; - if (error_number) *error_number = errno; - if (error_string) - archive_string_sprintf(error_string, - "Could not chdir %s", - path); + fsobj_error(a_eno, a_estr, errno, + "Could not chdir %s", path); res = (ARCHIVE_FATAL); break; } @@ -2514,11 +2526,9 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error */ if (unlink(head)) { tail[0] = c; - if (error_number) *error_number = errno; - if (error_string) - archive_string_sprintf(error_string, - "Could not remove symlink %s", - path); + fsobj_error(a_eno, a_estr, errno, + "Could not remove symlink %s", + path); res = ARCHIVE_FAILED; break; } @@ -2529,13 +2539,14 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error * symlink with another symlink. */ tail[0] = c; - /* FIXME: not sure how important this is to restore + /* + * FIXME: not sure how important this is to + * restore + */ + /* if (!S_ISLNK(path)) { - if (error_number) *error_number = 0; - if (error_string) - archive_string_sprintf(error_string, - "Removing symlink %s", - path); + fsobj_error(a_eno, a_estr, 0, + "Removing symlink %s", path); } */ /* Symlink gone. No more problem! */ @@ -2545,22 +2556,17 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error /* User asked us to remove problems. */ if (unlink(head) != 0) { tail[0] = c; - if (error_number) *error_number = 0; - if (error_string) - archive_string_sprintf(error_string, - "Cannot remove intervening symlink %s", - path); + fsobj_error(a_eno, a_estr, 0, + "Cannot remove intervening " + "symlink %s", path); res = ARCHIVE_FAILED; break; } tail[0] = c; } else { tail[0] = c; - if (error_number) *error_number = 0; - if (error_string) - archive_string_sprintf(error_string, - "Cannot extract through symlink %s", - path); + fsobj_error(a_eno, a_estr, 0, + "Cannot extract through symlink %s", path); res = ARCHIVE_FAILED; break; } @@ -2577,10 +2583,8 @@ check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error if (restore_pwd >= 0) { r = fchdir(restore_pwd); if (r != 0) { - if(error_number) *error_number = errno; - if(error_string) - archive_string_sprintf(error_string, - "chdir() failure"); + fsobj_error(a_eno, a_estr, errno, + "chdir() failure", ""); } close(restore_pwd); restore_pwd = -1; @@ -2688,17 +2692,16 @@ cleanup_pathname_win(struct archive_write_disk *a) * is set) if the path is absolute. */ static int -cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags) +cleanup_pathname_fsobj(char *path, int *a_eno, struct archive_string *a_estr, + int flags) { char *dest, *src; char separator = '\0'; dest = src = path; if (*src == '\0') { - if (error_number) *error_number = ARCHIVE_ERRNO_MISC; - if (error_string) - archive_string_sprintf(error_string, - "Invalid empty pathname"); + fsobj_error(a_eno, a_estr, ARCHIVE_ERRNO_MISC, + "Invalid empty ", "pathname"); return (ARCHIVE_FAILED); } @@ -2708,10 +2711,8 @@ cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *err /* Skip leading '/'. */ if (*src == '/') { if (flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) { - if (error_number) *error_number = ARCHIVE_ERRNO_MISC; - if (error_string) - archive_string_sprintf(error_string, - "Path is absolute"); + fsobj_error(a_eno, a_estr, ARCHIVE_ERRNO_MISC, + "Path is ", "absolute"); return (ARCHIVE_FAILED); } @@ -2738,11 +2739,11 @@ cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *err } else if (src[1] == '.') { if (src[2] == '/' || src[2] == '\0') { /* Conditionally warn about '..' */ - if (flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) { - if (error_number) *error_number = ARCHIVE_ERRNO_MISC; - if (error_string) - archive_string_sprintf(error_string, - "Path contains '..'"); + if (flags + & ARCHIVE_EXTRACT_SECURE_NODOTDOT) { + fsobj_error(a_eno, a_estr, + ARCHIVE_ERRNO_MISC, + "Path contains ", "'..'"); return (ARCHIVE_FAILED); } } -- 2.7.4