- guenther@cvs.openbsd.org 2009/12/20 07:28:36

     [ssh.c sftp.c scp.c]
     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@

diff --git a/ChangeLog b/ChangeLog
index 45f7585299e4b35d24479de5e53f5499b704a9b0..605e0dca775b619143c77e22e379312236157320 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -108,6 +108,17 @@
      [key.c]
      switch from 35 to the more common value of RSA_F4 == (2**16)+1 == 65537
      for the RSA public exponent; discussed with provos; ok djm@
+   - guenther@cvs.openbsd.org 2009/12/20 07:28:36
+     [ssh.c sftp.c scp.c]
+     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@
 
 20091226
  - (tim) [contrib/cygwin/Makefile] Install ssh-copy-id and ssh-copy-id.1

diff --git a/scp.c b/scp.c
index 323747806874cff09f86f9dd6795703141b0be8e..09efb82acb3e91b798a925ffbd932d2813eb44b3 100644 (file)
--- a/scp.c
+++ b/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).
@@ -244,8 +244,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);
 
@@ -337,10 +340,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");
@@ -548,6 +553,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);
@@ -558,7 +564,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)
@@ -591,6 +597,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))
@@ -610,7 +617,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/sftp.c b/sftp.c
index 1aa37423cba36932f9fa47e482c197eb3867a1f6..d8728cc25e0c94a9393a7eac32f5ebb682b68d27 100644 (file)
--- a/sftp.c
+++ b/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>
  *
@@ -1809,7 +1809,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) {
@@ -1829,6 +1830,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/ssh.c b/ssh.c
index 90dbc69e93dfb99ae4185ed0f802b95d5be50b14..6abf31b527d070e069f98c1911c1dc7ea990f289 100644 (file)
--- a/ssh.c
+++ b/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
@@ -528,7 +528,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, '@');

diff --git a/sshd_config.5 b/sshd_config.5
index a6a3f0a4b7e8a5845d91b0cfdc89eb3f2a82f253..d4d47b33a84782ab284f31855254df511f55850e 100644 (file)
--- a/sshd_config.5
+++ b/sshd_config.5
@@ -34,7 +34,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: sshd_config.5,v 1.112 2009/11/10 02:58:56 djm Exp $
+.\" $OpenBSD: sshd_config.5,v 1.113 2009/12/19 16:53:13 stevesk Exp $
 .Dd $Mdocdate$
 .Dt SSHD_CONFIG 5
 .Os
@@ -182,16 +182,16 @@ PAM or though authentication styles supported in
 The default is
 .Dq yes .
 .It Cm ChrootDirectory
-Specifies a path to
+Specifies the pathname of a directory to
 .Xr chroot 2
 to after authentication.
-This path, and all its components, must be root-owned directories that are
+All components of the pathname must be root-owned directories that are
 not writable by any other user or group.
 After the chroot,
 .Xr sshd 8
 changes the working directory to the user's home directory.
 .Pp
-The path may contain the following tokens that are expanded at runtime once
+The pathname may contain the following tokens that are expanded at runtime once
 the connecting user has been authenticated: %% is replaced by a literal '%',
 %h is replaced by the home directory of the user being authenticated, and
 %u is replaced by the username of that user.