From 6f58be07485fd2fbac1b828e5f2c631e7d5bc9e4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 5 Jul 2009 13:24:17 +0200 Subject: [PATCH] cp: make "non-POSIX" cp a bit more consistent Signed-off-by: Denys Vlasenko --- libbb/copy_file.c | 62 +++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/libbb/copy_file.c b/libbb/copy_file.c index 4d2f17aa5..ae70cbc0a 100644 --- a/libbb/copy_file.c +++ b/libbb/copy_file.c @@ -6,7 +6,6 @@ * SELinux support by Yuichi Nakamura * * Licensed under GPLv2 or later, see file LICENSE in this tarball for details. - * */ #include "libbb.h" @@ -16,31 +15,33 @@ // 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! +// If dest is a (sym)link, we overwrite link 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... -// -// NB: we have special code which still allows for "cp file /dev/node" -// to work POSIX-ly (the only realistic case where it makes sense) -// errno must be set to relevant value ("why we cannot create dest?") -// for POSIX mode to give reasonable error message +/* Called if open of destination, link creation etc fails. + * errno must be set to relevant value ("why we cannot create dest?") + * to give reasonable error message */ static int ask_and_unlink(const char *dest, int flags) { int e = errno; + #if !ENABLE_FEATURE_NON_POSIX_CP if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) { - // Either it exists, or the *path* doesnt exist + /* Either it exists, or the *path* doesnt exist */ bb_perror_msg("cannot create '%s'", dest); return -1; } #endif - // If ENABLE_FEATURE_NON_POSIX_CP, act as if -f is always in effect - // - we don't want "cannot create" msg, we want unlink to be done - // (silently unless -i). + // else: act as if -f is always in effect. + // We don't want "cannot create" msg, we want unlink to be done + // (silently unless -i). Why? POSIX cp usually succeeds with + // O_TRUNC open of existing file, and user is left ignorantly happy. + // With above block unconditionally enabled, non-POSIX cp + // will complain a lot more than POSIX one. - // TODO: maybe we should do it only if ctty is present? + /* 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. @@ -48,7 +49,7 @@ static int ask_and_unlink(const char *dest, int flags) // 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 + return 0; /* not allowed to overwrite */ } if (unlink(dest) < 0) { #if ENABLE_FEATURE_VERBOSE_CP_MESSAGE @@ -56,14 +57,14 @@ static int ask_and_unlink(const char *dest, int flags) /* e == ENOTDIR is similar: path has non-dir component, * but in this case we don't even reach copy_file() */ bb_error_msg("cannot create '%s': Path does not exist", dest); - return -1; // error + return -1; /* error */ } #endif - errno = e; + errno = e; /* do not use errno from unlink */ bb_perror_msg("cannot create '%s'", dest); - return -1; // error + return -1; /* error */ } - return 1; // ok (to try again) + return 1; /* ok (to try again) */ } /* Return: @@ -85,8 +86,8 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags) #define FLAGS_DEREF (flags & FILEUTILS_DEREFERENCE) if ((FLAGS_DEREF ? stat : lstat)(source, &source_stat) < 0) { - // This may be a dangling symlink. - // Making [sym]links to dangling symlinks works, so... + /* This may be a dangling symlink. + * Making [sym]links to dangling symlinks works, so... */ if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) goto make_links; bb_perror_msg("cannot stat '%s'", source); @@ -212,9 +213,9 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags) if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) { int (*lf)(const char *oldpath, const char *newpath); make_links: - // Hmm... maybe - // if (DEREF && MAKE_SOFTLINK) source = realpath(source) ? - // (but realpath returns NULL on dangling symlinks...) + /* Hmm... maybe + * if (DEREF && MAKE_SOFTLINK) source = realpath(source) ? + * (but realpath returns NULL on dangling symlinks...) */ lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link; if (lf(source, dest) < 0) { ovr = ask_and_unlink(dest, flags); @@ -226,7 +227,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags) } } /* _Not_ jumping to preserve_mode_ugid_time: - * hard/softlinks don't have those */ + * (sym)links don't have those */ return 0; } @@ -274,19 +275,12 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags) if (!S_ISREG(source_stat.st_mode)) new_mode = 0666; - /* POSIX way is a security problem versus symlink attacks, - * we do it only for non-symlinks, and only for non-recursive, - * non-interactive cp. NB: it is still racy - * for "cp file /home/bad_user/file" case - * (user can rm file and create a link to /etc/passwd) */ - if (!ENABLE_FEATURE_NON_POSIX_CP - || (dest_exists - && !(flags & (FILEUTILS_RECUR|FILEUTILS_INTERACTIVE)) - && !S_ISLNK(dest_stat.st_mode)) - ) { + // POSIX way is a security problem versus (sym)link attacks + if (!ENABLE_FEATURE_NON_POSIX_CP) { dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, new_mode); - } else /* safe way: */ + } else { /* safe way: */ dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode); + } if (dst_fd == -1) { ovr = ask_and_unlink(dest, flags); if (ovr <= 0) { -- 2.25.1