diff options
| -rw-r--r-- | libbb/copy_file.c | 78 |
1 files changed, 50 insertions, 28 deletions
diff --git a/libbb/copy_file.c b/libbb/copy_file.c index 636fbdc1d..58d657b02 100644 --- a/libbb/copy_file.c +++ b/libbb/copy_file.c @@ -11,13 +11,37 @@ #include "libbb.h" -static int retry_overwrite(const char *dest, int flags) +// POSIX: if exists and -i, ask (w/o -i assume yes). +// Then open w/o EXCL (yes, not unlink!). +// If open still fails and -f, try unlink, then try open again. +// Result: a mess: +// If dest is a softlink, we overwrite softlink's destination! +// (or fail, if it points to dir/nonexistent location/etc). +// This is strange, but POSIX-correct. +// coreutils cp has --remove-destination to override this... + +#define DO_POSIX_CP 0 /* 1 - POSIX behavior, 0 - safe behavior */ + + +static int ask_and_unlink(const char *dest, int flags) { + // If !DO_POSIX_CP, act as if -f is always in effect - we don't want + // "'file' exists" msg, we want unlink to be done (silently unless -i + // is also in effect). + // This prevents safe way from asking more questions than POSIX does. +#if DO_POSIX_CP if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) { fprintf(stderr, "'%s' exists\n", dest); return -1; } +#endif + + // TODO: maybe we should do it only if ctty is present? if (flags & FILEUTILS_INTERACTIVE) { + // We would not do POSIX insanity. -i asks, + // then _unlinks_ the offender. Presto. + // (No opening without O_EXCL, no unlinks only if -f) + // Or else we will end up having 3 open()s! fprintf(stderr, "%s: overwrite '%s'? ", applet_name, dest); if (!bb_ask_confirmation()) return 0; // not allowed to overwrite @@ -29,6 +53,11 @@ static int retry_overwrite(const char *dest, int flags) return 1; // ok (to try again) } +/* Return: + * -1 error, copy not made + * 0 copy is made or user answered "no" in interactive mode + * (failures to preserve mode/owner/times are not reported in exit code) + */ int copy_file(const char *source, const char *dest, int flags) { struct stat source_stat; @@ -72,13 +101,11 @@ int copy_file(const char *source, const char *dest, int flags) freecon(con); return -1; } + } else if (errno == ENOTSUP || errno == ENODATA) { + setfscreatecon_or_die(NULL); } else { - if (errno == ENOTSUP || errno == ENODATA) { - setfscreatecon_or_die(NULL); - } else { - bb_perror_msg("cannot lgetfilecon %s", source); - return -1; - } + bb_perror_msg("cannot lgetfilecon %s", source); + return -1; } } #endif @@ -153,7 +180,7 @@ int copy_file(const char *source, const char *dest, int flags) // (but realpath returns NULL on dangling symlinks...) lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link; if (lf(source, dest) < 0) { - ovr = retry_overwrite(dest, flags); + ovr = ask_and_unlink(dest, flags); if (ovr <= 0) return ovr; if (lf(source, dest) < 0) { @@ -164,8 +191,8 @@ int copy_file(const char *source, const char *dest, int flags) return 0; } else if (S_ISREG(source_stat.st_mode) - // Huh? DEREF uses stat, which never returns links IIRC... - || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) + /* Huh? DEREF uses stat, which never returns links! */ + /* || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) */ ) { int src_fd; int dst_fd; @@ -176,7 +203,7 @@ int copy_file(const char *source, const char *dest, int flags) link_name = is_in_ino_dev_hashtable(&source_stat); if (link_name) { if (link(link_name, dest) < 0) { - ovr = retry_overwrite(dest, flags); + ovr = ask_and_unlink(dest, flags); if (ovr <= 0) return ovr; if (link(link_name, dest) < 0) { @@ -196,27 +223,21 @@ int copy_file(const char *source, const char *dest, int flags) return -1; } - // POSIX: if exists and -i, ask (w/o -i assume yes). - // Then open w/o EXCL. - // If open still fails and -f, try unlink, then try open again. - // Result: a mess: - // If dest is a softlink, we overwrite softlink's destination! - // (or fail, if it points to dir/nonexistent location/etc). - // This is strange, but POSIX-correct. - // coreutils cp has --remove-destination to override this... +#if DO_POSIX_CP /* POSIX way (a security problem versus symlink attacks!): */ dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE) - ? O_WRONLY|O_CREAT|O_TRUNC|O_EXCL + ? O_WRONLY|O_CREAT|O_EXCL : O_WRONLY|O_CREAT|O_TRUNC, source_stat.st_mode); +#else /* safe way: */ + dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, source_stat.st_mode); +#endif if (dst_fd == -1) { - // We would not do POSIX insanity. -i asks, - // then _unlinks_ the offender. Presto. - // Or else we will end up having 3 open()s! - ovr = retry_overwrite(dest, flags); + ovr = ask_and_unlink(dest, flags); if (ovr <= 0) { close(src_fd); return ovr; } - dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, source_stat.st_mode); + /* It shouldn't exist. If it exists, do not open (symlink attack?) */ + dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, source_stat.st_mode); if (dst_fd == -1) { bb_perror_msg("cannot open '%s'", dest); close(src_fd); @@ -227,7 +248,8 @@ int copy_file(const char *source, const char *dest, int flags) #if ENABLE_SELINUX if (((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT) || (flags & FILEUTILS_SET_SECURITY_CONTEXT)) - && is_selinux_enabled() > 0) { + && is_selinux_enabled() > 0 + ) { security_context_t con; if (getfscreatecon(&con) == -1) { bb_perror_msg("getfscreatecon"); @@ -260,7 +282,7 @@ int copy_file(const char *source, const char *dest, int flags) ) { // We are lazy here, a bit lax with races... if (dest_exists) { - ovr = retry_overwrite(dest, flags); + ovr = ask_and_unlink(dest, flags); if (ovr <= 0) return ovr; } @@ -273,7 +295,7 @@ int copy_file(const char *source, const char *dest, int flags) char *lpath; lpath = xmalloc_readlink_or_warn(source); - if (symlink(lpath, dest) < 0) { + if (lpath && symlink(lpath, dest) < 0) { bb_perror_msg("cannot create symlink '%s'", dest); free(lpath); return -1; |