Apple-2gs-SW/hush
1
0
Fork 0
mirror of https://github.com/sheumann/hush.git synced 2024-12-22 14:30:31 +00:00
Code Issues Projects Releases Wiki Activity

copy_file: comment out one condition which is always false.

Browse Source 
Add comment explaining POSIX rules for cp - and why
these rules are dangerous. Provide conditionally compiled code
for both POSIX and safe behaviors, select safe for now.
Code shrunk by ~80 bytes.
This commit is contained in:
Denis Vlasenko 2007-03-15 13:33:37 +00:00
parent 24af7201e9
commit 54d14ca1a2
1 changed files with 50 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
libbb/copy_file.c
View File

@ -11,13 +11,37 @@
#include "libbb.h" #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))) { if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) {
fprintf(stderr, "'%s' exists\n", dest); fprintf(stderr, "'%s' exists\n", dest);
return -1; return -1;
} }
#endif
// TODO: maybe we should do it only if ctty is present?
if (flags & FILEUTILS_INTERACTIVE) { 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); fprintf(stderr, "%s: overwrite '%s'? ", applet_name, dest);
if (!bb_ask_confirmation()) if (!bb_ask_confirmation())
return 0; // not allowed to overwrite 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; // 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) int copy_file(const char *source, const char *dest, int flags)
{ {
struct stat source_stat; struct stat source_stat;
@ -72,13 +101,11 @@ int copy_file(const char *source, const char *dest, int flags)
freecon(con); freecon(con);
return -1; return -1;
} }
} else if (errno == ENOTSUP || errno == ENODATA) {
setfscreatecon_or_die(NULL);
} else { } else {
if (errno == ENOTSUP || errno == ENODATA) { bb_perror_msg("cannot lgetfilecon %s", source);
setfscreatecon_or_die(NULL); return -1;
} else {
bb_perror_msg("cannot lgetfilecon %s", source);
return -1;
}
} }
} }
#endif #endif
@ -153,7 +180,7 @@ int copy_file(const char *source, const char *dest, int flags)
// (but realpath returns NULL on dangling symlinks...) // (but realpath returns NULL on dangling symlinks...)
lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link; lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link;
if (lf(source, dest) < 0) { if (lf(source, dest) < 0) {
ovr = retry_overwrite(dest, flags); ovr = ask_and_unlink(dest, flags);
if (ovr <= 0) if (ovr <= 0)
return ovr; return ovr;
if (lf(source, dest) < 0) { if (lf(source, dest) < 0) {
@ -164,8 +191,8 @@ int copy_file(const char *source, const char *dest, int flags)
return 0; return 0;
} else if (S_ISREG(source_stat.st_mode) } else if (S_ISREG(source_stat.st_mode)
// Huh? DEREF uses stat, which never returns links IIRC... /* Huh? DEREF uses stat, which never returns links! */
|| (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) /* || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) */
) { ) {
int src_fd; int src_fd;
int dst_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); link_name = is_in_ino_dev_hashtable(&source_stat);
if (link_name) { if (link_name) {
if (link(link_name, dest) < 0) { if (link(link_name, dest) < 0) {
ovr = retry_overwrite(dest, flags); ovr = ask_and_unlink(dest, flags);
if (ovr <= 0) if (ovr <= 0)
return ovr; return ovr;
if (link(link_name, dest) < 0) { if (link(link_name, dest) < 0) {
@ -196,27 +223,21 @@ int copy_file(const char *source, const char *dest, int flags)
return -1; return -1;
} }
// POSIX: if exists and -i, ask (w/o -i assume yes). #if DO_POSIX_CP /* POSIX way (a security problem versus symlink attacks!): */
// 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...
dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE) 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); : 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) { if (dst_fd == -1) {
// We would not do POSIX insanity. -i asks, ovr = ask_and_unlink(dest, flags);
// then _unlinks_ the offender. Presto.
// Or else we will end up having 3 open()s!
ovr = retry_overwrite(dest, flags);
if (ovr <= 0) { if (ovr <= 0) {
close(src_fd); close(src_fd);
return ovr; 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) { if (dst_fd == -1) {
bb_perror_msg("cannot open '%s'", dest); bb_perror_msg("cannot open '%s'", dest);
close(src_fd); close(src_fd);
@ -227,7 +248,8 @@ int copy_file(const char *source, const char *dest, int flags)
#if ENABLE_SELINUX #if ENABLE_SELINUX
if (((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT) if (((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT)
|| (flags & FILEUTILS_SET_SECURITY_CONTEXT)) || (flags & FILEUTILS_SET_SECURITY_CONTEXT))
&& is_selinux_enabled() > 0) { && is_selinux_enabled() > 0
) {
security_context_t con; security_context_t con;
if (getfscreatecon(&con) == -1) { if (getfscreatecon(&con) == -1) {
bb_perror_msg("getfscreatecon"); 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... // We are lazy here, a bit lax with races...
if (dest_exists) { if (dest_exists) {
ovr = retry_overwrite(dest, flags); ovr = ask_and_unlink(dest, flags);
if (ovr <= 0) if (ovr <= 0)
return ovr; return ovr;
} }
@ -273,7 +295,7 @@ int copy_file(const char *source, const char *dest, int flags)
char *lpath; char *lpath;
lpath = xmalloc_readlink_or_warn(source); 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); bb_perror_msg("cannot create symlink '%s'", dest);
free(lpath); free(lpath);
return -1; return -1;