Bug: Copying several files to non-directory.

[Björn Augustsson]

On Fri, Jan 20, 2006 at 01:17:22PM +0100, Björn Augustsson wrote:
> Dear openssh people,
> 
> Here's something to try:
> 
> $ echo a >a
> $ echo b >b
> $ echo c >c
> $
> $ scp a b c
> $
> $ echo $?
> 1
> $ cat a b c
> a
> b
> b
> 
> Ouch!  Also, for comparison:
> 
> $ cp a b c
> cp: copying multiple files, but last argument `c' is not a directory
> Try `cp --help' for more information.
> $ rcp a b c
> rcp: c: Not a directory.
> $
> 
> Note that the cp behavior is specified in SuSv2.
> (http://www.opengroup.org/onlinepubs/007908799/xcu/cp.html, "Second synopsis")
> 
> Also, and this might be a separate bug, note how openssh exits with
> return code of 1.
> 
> This was verified using various versions of scp from 3.6.1p2 to 4.2, on 
> linux (various versions) and openbsd.

Here's a patch that seems to fix it. (Against 4.2p1) 

The first chunk fixed the eating of the innocent file, and is fairly
obvoiusly correct. The second one fixes the fact that there's no error
message. I'm less certain of that one not breaking anything else. 
(A colleague wrote that one... :)

/August.


diff -u openssh-4.2p1/scp.c openssh-4.2p1-hack/scp.c
--- openssh-4.2p1/scp.c	2005-08-02 09:07:08.000000000 +0200
+++ openssh-4.2p1-hack/scp.c	2006-01-23 17:31:37.000000000 +0100
@@ -336,9 +336,10 @@
 	if ((targ = colon(argv[argc - 1])))	/* Dest is remote host. */
 		toremote(targ, argc, argv);
 	else {
-		tolocal(argc, argv);	/* Dest is local host. */
+		/* Dest is local host. */
 		if (targetshouldbedirectory)
 			verifydir(argv[argc - 1]);
+		tolocal(argc, argv);
 	}
 	/*
 	 * Finally check the exit status of the ssh process, if one was forked
@@ -1045,16 +1046,15 @@
 	va_list ap;
 
 	++errs;
-	if (fp == NULL && !(fp = fdopen(remout, "w")))
-		return;
-	(void) fprintf(fp, "%c", 0x01);
-	(void) fprintf(fp, "scp: ");
-	va_start(ap, fmt);
-	(void) vfprintf(fp, fmt, ap);
-	va_end(ap);
-	(void) fprintf(fp, "\n");
-	(void) fflush(fp);
-
+	if (fp != NULL || (fp = fdopen(remout, "w"))) {
+		(void) fprintf(fp, "%c", 0x01);
+		(void) fprintf(fp, "scp: ");
+		va_start(ap, fmt);
+		(void) vfprintf(fp, fmt, ap);
+		va_end(ap);
+		(void) fprintf(fp, "\n");
+		(void) fflush(fp);
+	}
 	if (!iamremote) {
 		va_start(ap, fmt);
 		vfprintf(stderr, fmt, ap);
-- 
Björn Augustsson      Sysadmin     IT Systems & Services
Chalmers tekniska högskola     Chalmers University of Technology
         "Damn spooky analog crap." -- John Carmack.