When passing user-controlled options with arguments to other programs,

pass the option and option argument as separate argv entries and
not smashed into one (e.g., as -l foo and not -lfoo).  Also, always
pass a "--" argument to stop option parsing, so that a positional
argument that starts with a '-' isn't treated as an option.  This
fixes some error cases as well as the handling of hostnames and
filenames that start with a '-'.

Based on a diff by halex@
ok halex@ djm@ deraadt@

3 files changed, 20 insertions, 11 deletions

diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index 2ecc06616cb..7449ec46539 100644
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.164 2008/10/10 04:55:16 stevesk Exp $ */
+/* $OpenBSD: scp.c,v 1.165 2009/12/20 07:28:36 guenther Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -228,8 +228,11 @@ do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout)
 		close(pout[1]);
 
 		replacearg(&args, 0, "%s", ssh_program);
-		if (remuser != NULL)
-			addargs(&args, "-l%s", remuser);
+		if (remuser != NULL) {
+			addargs(&args, "-l");
+			addargs(&args, "%s", remuser);
+		}
+		addargs(&args, "--");
 		addargs(&args, "%s", host);
 		addargs(&args, "%s", cmd);
 
@@ -318,10 +321,12 @@ main(int argc, char **argv)
 		case 'c':
 		case 'i':
 		case 'F':
-			addargs(&args, "-%c%s", ch, optarg);
+			addargs(&args, "-%c", ch);
+			addargs(&args, "%s", optarg);
 			break;
 		case 'P':
-			addargs(&args, "-p%s", optarg);
+			addargs(&args, "-p");
+			addargs(&args, "%s", optarg);
 			break;
 		case 'B':
 			addargs(&args, "-oBatchmode yes");
@@ -526,6 +531,7 @@ toremote(char *targ, int argc, char **argv)
 			} else {
 				host = cleanhostname(argv[i]);
 			}
+			addargs(&alist, "--");
 			addargs(&alist, "%s", host);
 			addargs(&alist, "%s", cmd);
 			addargs(&alist, "%s", src);
@@ -536,7 +542,7 @@ toremote(char *targ, int argc, char **argv)
 				errs = 1;
 		} else {	/* local to remote */
 			if (remin == -1) {
-				xasprintf(&bp, "%s -t %s", cmd, targ);
+				xasprintf(&bp, "%s -t -- %s", cmd, targ);
 				host = cleanhostname(thost);
 				if (do_cmd(host, tuser, bp, &remin,
 				    &remout) < 0)
@@ -569,6 +575,7 @@ tolocal(int argc, char **argv)
 				addargs(&alist, "-r");
 			if (pflag)
 				addargs(&alist, "-p");
+			addargs(&alist, "--");
 			addargs(&alist, "%s", argv[i]);
 			addargs(&alist, "%s", argv[argc-1]);
 			if (do_local_cmd(&alist))
@@ -588,7 +595,7 @@ tolocal(int argc, char **argv)
 				suser = pwd->pw_name;
 		}
 		host = cleanhostname(host);
-		xasprintf(&bp, "%s -f %s", cmd, src);
+		xasprintf(&bp, "%s -f -- %s", cmd, src);
 		if (do_cmd(host, suser, bp, &remin, &remout) < 0) {
 			(void) xfree(bp);
 			++errs;
diff --git a/usr.bin/ssh/sftp.c b/usr.bin/ssh/sftp.c
index d572fb168c6..dcf8136053e 100644
--- a/usr.bin/ssh/sftp.c
+++ b/usr.bin/ssh/sftp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp.c,v 1.114 2009/12/06 23:53:54 dtucker Exp $ */
+/* $OpenBSD: sftp.c,v 1.115 2009/12/20 07:28:36 guenther Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -1760,7 +1760,8 @@ main(int argc, char **argv)
 				fprintf(stderr, "Missing username\n");
 				usage();
 			}
-			addargs(&args, "-l%s", userhost);
+			addargs(&args, "-l");
+			addargs(&args, "%s", userhost);
 		}
 
 		if ((cp = colon(host)) != NULL) {
@@ -1780,6 +1781,7 @@ main(int argc, char **argv)
 		if (sftp_server == NULL || strchr(sftp_server, '/') == NULL)
 			addargs(&args, "-s");
 
+		addargs(&args, "--");
 		addargs(&args, "%s", host);
 		addargs(&args, "%s", (sftp_server != NULL ?
 		    sftp_server : "sftp"));
diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c
index 80ac30d4558..b3e3e1c0b88 100644
--- a/usr.bin/ssh/ssh.c
+++ b/usr.bin/ssh/ssh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.328 2009/10/28 16:38:18 reyk Exp $ */
+/* $OpenBSD: ssh.c,v 1.329 2009/12/20 07:28:36 guenther Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -515,7 +515,7 @@ main(int ac, char **av)
 	ac -= optind;
 	av += optind;
 
-	if (ac > 0 && !host && **av != '-') {
+	if (ac > 0 && !host) {
 		if (strrchr(*av, '@')) {
 			p = xstrdup(*av);
 			cp = strrchr(p, '@');