diff options
author | Damien Miller <djm@cvs.openbsd.org> | 2006-01-31 10:19:03 +0000 |
---|---|---|
committer | Damien Miller <djm@cvs.openbsd.org> | 2006-01-31 10:19:03 +0000 |
commit | 4642520f044beb240350338ee00862f58a91200b (patch) | |
tree | e9389c74fca3c0fe8187ec99fe239d1ecd6f61ca /usr.bin | |
parent | efd59c8464a11bdf906696175b911ed746ad30e5 (diff) |
fix local arbitrary command execution vulnerability on local/local and
remote/remote copies (CVE-2006-0225, bz #1094), patch by
t8m AT centrum.cz, polished by dtucker@ and myself; ok markus@
Diffstat (limited to 'usr.bin')
-rw-r--r-- | usr.bin/ssh/misc.c | 45 | ||||
-rw-r--r-- | usr.bin/ssh/misc.h | 8 | ||||
-rw-r--r-- | usr.bin/ssh/scp.c | 132 | ||||
-rw-r--r-- | usr.bin/ssh/sftp.c | 8 |
4 files changed, 139 insertions, 54 deletions
diff --git a/usr.bin/ssh/misc.c b/usr.bin/ssh/misc.c index 5a990db232e..9ec397aee8a 100644 --- a/usr.bin/ssh/misc.c +++ b/usr.bin/ssh/misc.c @@ -24,7 +24,7 @@ */ #include "includes.h" -RCSID("$OpenBSD: misc.c,v 1.41 2006/01/05 23:43:53 djm Exp $"); +RCSID("$OpenBSD: misc.c,v 1.42 2006/01/31 10:19:02 djm Exp $"); #include <net/if.h> @@ -383,12 +383,15 @@ void addargs(arglist *args, char *fmt, ...) { va_list ap; - char buf[1024]; + char *cp; u_int nalloc; + int r; va_start(ap, fmt); - vsnprintf(buf, sizeof(buf), fmt, ap); + r = vasprintf(&cp, fmt, ap); va_end(ap); + if (r == -1) + fatal("addargs: argument too long"); nalloc = args->nalloc; if (args->list == NULL) { @@ -399,10 +402,44 @@ addargs(arglist *args, char *fmt, ...) args->list = xrealloc(args->list, nalloc * sizeof(char *)); args->nalloc = nalloc; - args->list[args->num++] = xstrdup(buf); + args->list[args->num++] = cp; args->list[args->num] = NULL; } +void +replacearg(arglist *args, u_int which, char *fmt, ...) +{ + va_list ap; + char *cp; + int r; + + va_start(ap, fmt); + r = vasprintf(&cp, fmt, ap); + va_end(ap); + if (r == -1) + fatal("replacearg: argument too long"); + + if (which >= args->num) + fatal("replacearg: tried to replace invalid arg %d >= %d", + which, args->num); + xfree(args->list[which]); + args->list[which] = cp; +} + +void +freeargs(arglist *args) +{ + u_int i; + + if (args->list != NULL) { + for (i = 0; i < args->num; i++) + xfree(args->list[i]); + xfree(args->list); + args->nalloc = args->num = 0; + args->list = NULL; + } +} + /* * Expands tildes in the file name. Returns data allocated by xmalloc. * Warning: this calls getpw*. diff --git a/usr.bin/ssh/misc.h b/usr.bin/ssh/misc.h index 41591068695..0a1a09a68ba 100644 --- a/usr.bin/ssh/misc.h +++ b/usr.bin/ssh/misc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.28 2005/12/08 18:34:11 reyk Exp $ */ +/* $OpenBSD: misc.h,v 1.29 2006/01/31 10:19:02 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> @@ -38,7 +38,11 @@ struct arglist { u_int num; u_int nalloc; }; -void addargs(arglist *, char *, ...) __attribute__((format(printf, 2, 3))); +void addargs(arglist *, char *, ...) + __attribute__((format(printf, 2, 3))); +void replacearg(arglist *, u_int, char *, ...) + __attribute__((format(printf, 3, 4))); +void freeargs(arglist *); /* readpass.c */ diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c index 1b0424f3065..bacefe17708 100644 --- a/usr.bin/ssh/scp.c +++ b/usr.bin/ssh/scp.c @@ -71,7 +71,7 @@ */ #include "includes.h" -RCSID("$OpenBSD: scp.c,v 1.128 2005/12/06 22:38:27 reyk Exp $"); +RCSID("$OpenBSD: scp.c,v 1.129 2006/01/31 10:19:02 djm Exp $"); #include "xmalloc.h" #include "atomicio.h" @@ -118,6 +118,48 @@ killchild(int signo) exit(1); } +static int +do_local_cmd(arglist *a) +{ + u_int i; + int status; + pid_t pid; + + if (a->num == 0) + fatal("do_local_cmd: no arguments"); + + if (verbose_mode) { + fprintf(stderr, "Executing:"); + for (i = 0; i < a->num; i++) + fprintf(stderr, " %s", a->list[i]); + fprintf(stderr, "\n"); + } + if ((pid = fork()) == -1) + fatal("do_local_cmd: fork: %s", strerror(errno)); + + if (pid == 0) { + execvp(a->list[0], a->list); + perror(a->list[0]); + exit(1); + } + + do_cmd_pid = pid; + signal(SIGTERM, killchild); + signal(SIGINT, killchild); + signal(SIGHUP, killchild); + + while (waitpid(pid, &status, 0) == -1) + if (errno != EINTR) + fatal("do_local_cmd: waitpid: %s", strerror(errno)); + + do_cmd_pid = -1; + + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) + return (-1); + + return (0); +} + /* * This function executes the given command as the specified user on the * given host. This returns < 0 if execution fails, and >= 0 otherwise. This @@ -162,7 +204,7 @@ do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) close(pin[0]); close(pout[1]); - args.list[0] = ssh_program; + replacearg(&args, 0, "%s", ssh_program); if (remuser != NULL) addargs(&args, "-l%s", remuser); addargs(&args, "%s", host); @@ -225,8 +267,9 @@ main(int argc, char **argv) /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ sanitise_stdfd(); + memset(&args, '\0', sizeof(args)); args.list = NULL; - addargs(&args, "ssh"); /* overwritten with ssh_program */ + addargs(&args, "%s", ssh_program); addargs(&args, "-x"); addargs(&args, "-oForwardAgent no"); addargs(&args, "-oPermitLocalCommand no"); @@ -363,6 +406,10 @@ toremote(char *targ, int argc, char **argv) { int i, len; char *bp, *host, *src, *suser, *thost, *tuser, *arg; + arglist alist; + + memset(&alist, '\0', sizeof(alist)); + alist.list = NULL; *targ++ = 0; if (*targ == 0) @@ -380,56 +427,48 @@ toremote(char *targ, int argc, char **argv) tuser = NULL; } + if (tuser != NULL && !okname(tuser)) { + xfree(arg); + return; + } + for (i = 0; i < argc - 1; i++) { src = colon(argv[i]); if (src) { /* remote to remote */ - static char *ssh_options = - "-x -o'ClearAllForwardings yes'"; + freeargs(&alist); + addargs(&alist, "%s", ssh_program); + if (verbose_mode) + addargs(&alist, "-v"); + addargs(&alist, "-x"); + addargs(&alist, "-oClearAllForwardings yes"); + addargs(&alist, "-n"); + *src++ = 0; if (*src == 0) src = "."; host = strrchr(argv[i], '@'); - len = strlen(ssh_program) + strlen(argv[i]) + - strlen(src) + (tuser ? strlen(tuser) : 0) + - strlen(thost) + strlen(targ) + - strlen(ssh_options) + CMDNEEDS + 20; - bp = xmalloc(len); + if (host) { *host++ = 0; host = cleanhostname(host); suser = argv[i]; if (*suser == '\0') suser = pwd->pw_name; - else if (!okname(suser)) { - xfree(bp); + else if (!okname(suser)) continue; - } - if (tuser && !okname(tuser)) { - xfree(bp); - continue; - } - snprintf(bp, len, - "%s%s %s -n " - "-l %s %s %s %s '%s%s%s:%s'", - ssh_program, verbose_mode ? " -v" : "", - ssh_options, suser, host, cmd, src, - tuser ? tuser : "", tuser ? "@" : "", - thost, targ); + addargs(&alist, "-l"); + addargs(&alist, "%s", suser); } else { host = cleanhostname(argv[i]); - snprintf(bp, len, - "exec %s%s %s -n %s " - "%s %s '%s%s%s:%s'", - ssh_program, verbose_mode ? " -v" : "", - ssh_options, host, cmd, src, - tuser ? tuser : "", tuser ? "@" : "", - thost, targ); } - if (verbose_mode) - fprintf(stderr, "Executing: %s\n", bp); - if (system(bp) != 0) + addargs(&alist, "%s", host); + addargs(&alist, "%s", cmd); + addargs(&alist, "%s", src); + addargs(&alist, "%s%s%s:%s", + tuser ? tuser : "", tuser ? "@" : "", + thost, targ); + if (do_local_cmd(&alist) != 0) errs = 1; - (void) xfree(bp); } else { /* local to remote */ if (remin == -1) { len = strlen(targ) + CMDNEEDS + 20; @@ -453,20 +492,23 @@ tolocal(int argc, char **argv) { int i, len; char *bp, *host, *src, *suser; + arglist alist; + + memset(&alist, '\0', sizeof(alist)); + alist.list = NULL; for (i = 0; i < argc - 1; i++) { if (!(src = colon(argv[i]))) { /* Local to local. */ - len = strlen(_PATH_CP) + strlen(argv[i]) + - strlen(argv[argc - 1]) + 20; - bp = xmalloc(len); - (void) snprintf(bp, len, "exec %s%s%s %s %s", _PATH_CP, - iamrecursive ? " -r" : "", pflag ? " -p" : "", - argv[i], argv[argc - 1]); - if (verbose_mode) - fprintf(stderr, "Executing: %s\n", bp); - if (system(bp)) + freeargs(&alist); + addargs(&alist, "%s", _PATH_CP); + if (iamrecursive) + addargs(&alist, "-r"); + if (pflag) + addargs(&alist, "-p"); + addargs(&alist, "%s", argv[i]); + addargs(&alist, "%s", argv[argc-1]); + if (do_local_cmd(&alist)) ++errs; - (void) xfree(bp); continue; } *src++ = 0; diff --git a/usr.bin/ssh/sftp.c b/usr.bin/ssh/sftp.c index eb2b1941c97..52437f1f5be 100644 --- a/usr.bin/ssh/sftp.c +++ b/usr.bin/ssh/sftp.c @@ -16,7 +16,7 @@ #include "includes.h" -RCSID("$OpenBSD: sftp.c,v 1.69 2005/12/06 22:38:27 reyk Exp $"); +RCSID("$OpenBSD: sftp.c,v 1.70 2006/01/31 10:19:02 djm Exp $"); #include <glob.h> #include <histedit.h> @@ -1433,8 +1433,9 @@ main(int argc, char **argv) /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ sanitise_stdfd(); + memset(&args, '\0', sizeof(args)); args.list = NULL; - addargs(&args, "ssh"); /* overwritten with ssh_program */ + addargs(&args, ssh_program); addargs(&args, "-oForwardX11 no"); addargs(&args, "-oForwardAgent no"); addargs(&args, "-oPermitLocalCommand no"); @@ -1469,6 +1470,7 @@ main(int argc, char **argv) break; case 'S': ssh_program = optarg; + replacearg(&args, 0, "%s", ssh_program); break; case 'b': if (batchmode) @@ -1545,7 +1547,6 @@ main(int argc, char **argv) addargs(&args, "%s", host); addargs(&args, "%s", (sftp_server != NULL ? sftp_server : "sftp")); - args.list[0] = ssh_program; if (!batchmode) fprintf(stderr, "Connecting to %s...\n", host); @@ -1558,6 +1559,7 @@ main(int argc, char **argv) fprintf(stderr, "Attaching to %s...\n", sftp_direct); connect_to_server(sftp_direct, args.list, &in, &out); } + freeargs(&args); err = interactive_loop(in, out, file1, file2); |