From b8c884a0ba4050e4267be786414127c0f09d5544 Mon Sep 17 00:00:00 2001 From: Darren Tucker Date: Fri, 8 Jan 2010 18:53:43 +1100 Subject: [PATCH] - 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@ --- ChangeLog | 11 +++++++++++ scp.c | 21 ++++++++++++++------- sftp.c | 6 ++++-- ssh.c | 4 ++-- sshd_config.5 | 10 +++++----- 5 files changed, 36 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 45f758529..605e0dca7 100644 --- 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 323747806..09efb82ac 100644 --- 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 1aa37423c..d8728cc25 100644 --- 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 * @@ -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 90dbc69e9..6abf31b52 100644 --- 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 * Copyright (c) 1995 Tatu Ylonen , 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 e54e70079..6d2ad9df0 100644 --- a/sshd_config.5 +++ b/sshd_config.5 @@ -34,8 +34,8 @@ .\" (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 $ -.Dd $Mdocdate: November 10 2009 $ +.\" $OpenBSD: sshd_config.5,v 1.113 2009/12/19 16:53:13 stevesk Exp $ +.Dd $Mdocdate: December 19 2009 $ .Dt SSHD_CONFIG 5 .Os .Sh NAME @@ -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.