← Commit list

6f58be074

Denys Vlasenko

2009-07-05

FAILED Duration: 33s

Subject

cp: make "non-POSIX" cp a bit more consistent

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

libbb/copy_file.c +28 -34

libbb/copy_file.c+28 -34

@@ -6,7 +6,6 @@
	* SELinux support by Yuichi Nakamura <[email protected]>
	*
	* 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) {