On Tuesday 11 September 2007 00:19, Alexander Kriegisch wrote:
> In SVN rev. #18119 [1] the default behaviour of 'cp' was changed by
> Denis in order to improve security and also to save a few bytes. This
> leads to problems in our application scenario here: We work on a
> mipsel-based router platform equipped with a TFFS (tiny flash
> filesystem) showing several "files" as character devices. Copying new
> contents over existing files results in the character devices being
> unlinked and regular files being created instead, which is a problem
> because they are not saved back into the flash memory, thus the changes
> are not persistent. We have to use 'cat' in order to fix this.
>
> Denis, would you please consider making the hard-coded value of
> DO_POSIX_CP configurable? I could provide a patch, if needed, but it
> should be pretty straightforward and will probably be cleaner if you do
> it yourself. This way the default would still be the one you prefer and
> our firmware mod using a newer BB version than the original one would
> still work if we just changed this config option.
Try this, is it good enough?
--
vda
diff -d -urpN busybox.8/libbb/copy_file.c busybox.9/libbb/copy_file.c
--- busybox.8/libbb/copy_file.c 2007-09-06 20:05:34.000000000 +0100
+++ busybox.9/libbb/copy_file.c 2007-09-11 11:30:45.000000000 +0100
@@ -19,6 +19,9 @@
// (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)
#define DO_POSIX_CP 0 /* 1 - POSIX behavior, 0 - safe behavior */
@@ -243,13 +246,18 @@ int copy_file(const char *source, const
if (src_fd < 0)
return -1;
-#if DO_POSIX_CP /* POSIX way (a security problem versus symlink attacks!): */
- dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE)
- ? 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
+ /* POSIX way is a security problem versus symlink attacks,
+ * we do it only for dest's which are device nodes,
+ * and only for non-recursive, non-interactive cp. NB: it is still racy
+ * for "cp file /home/bad_user/device_node" case
+ * (user can rm device_node and create link to /etc/passwd) */
+ if (DO_POSIX_CP
+ || (dest_exists && !(flags & (FILEUTILS_RECUR|FILEUTILS_INTERACTIVE))
+ && (S_ISBLK(dest_stat.st_mode) || S_ISCHR(dest_stat.st_mode)))
+ ) {
+ dst_fd = open(dest, 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);
if (dst_fd == -1) {
ovr = ask_and_unlink(dest, flags);
if (ovr <= 0) {
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
