Date: Wed, 6 Jun 2001 14:33:23 +0300 From: Peter Pentchev <roam@orbitel.bg> To: Andreas Haugsnes <andreas@haugsnes.no> Cc: security@freebsd.org Subject: Re: [fwd] SSH allows deletion of other users files... Message-ID: <20010606143323.G18735@ringworld.oblivion.bg> In-Reply-To: <20010606131130.A26605@consistent.unicore.no>; from andreas@haugsnes.no on Wed, Jun 06, 2001 at 01:11:30PM %2B0200 References: <20010606124702.A30808@lucky.net> <20010606124822.A26583@consistent.unicore.no> <20010606125321.A56634@mithrandr.moria.org> <20010606131130.A26605@consistent.unicore.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jun 06, 2001 at 01:11:30PM +0200, Andreas Haugsnes wrote:
> Ahh, tested it now, yes, it is vulnerable.
> But wouldn't an easy workaround be too disable all X11-forwarding in sshd?
> I had it to 'no' here, but that was not per default.
>
>
> - Andreas Haugsnes
>
> On Wed, Jun 06, 2001 at 12:53:21PM +0200, Neil Blakey-Milner wrote:
> >
> > Are you using X forwarding? (ie, ssh -X)
Yes, disabling X forwarding would be an easy workaround.
Can somebody, however, test if the following patch resolves the problem?
It certainly does for me..
Well, ok, so there is still a race condition between the stat() and unlink()
in the cleanup procedure.. but since there is no funlink() yet, I do not
really think this one can be resolved :( And besides, there's a *much*
smaller window of opportunity there.
The attached patch is against RELENG_4, but it applies cleanly to the newer
OpenSSH in -current.
G'luck,
Peter
--
No language can express every thought unambiguously, least of all this one.
Index: src/crypto/openssh/session.c
===================================================================
RCS file: /home/ncvs/src/crypto/openssh/session.c,v
retrieving revision 1.4.2.8
diff -u -r1.4.2.8 session.c
--- src/crypto/openssh/session.c 2001/03/22 00:28:35 1.4.2.8
+++ src/crypto/openssh/session.c 2001/06/06 11:29:07
@@ -116,6 +116,7 @@
/* Local Xauthority file. */
static char *xauthfile;
+static struct stat xauthfile_sbuf;
/* original command from peer. */
char *original_command = NULL;
@@ -138,6 +139,33 @@
if (xauthfile != NULL) {
char *p;
+ struct stat sb;
+
+ if ((xauthfile_sbuf.st_dev != 0) ||
+ (xauthfile_sbuf.st_ino != 0)) {
+ debug("xauthfile sbuf/ino != 0, checking..");
+ if (stat(xauthfile, &sb) == -1) {
+ error("cannot stat xauthfile %s: %s",
+ xauthfile, strerror(errno));
+ /* bail out, do not remove! */
+ xfree(xauthfile);
+ xauthfile = NULL;
+ return;
+ }
+ if ((sb.st_dev != xauthfile_sbuf.st_dev) ||
+ (sb.st_ino != xauthfile_sbuf.st_ino)) {
+ error("xauthfile dev/ino mismatch! "
+ "expected: %lu/%lu, got %lu/%lu",
+ xauthfile_sbuf.st_dev,
+ xauthfile_sbuf.st_ino,
+ sb.st_dev, sb.st_ino);
+ /* bail out, do not remove! */
+ xfree(xauthfile);
+ xauthfile = NULL;
+ return;
+ }
+ debug("sbuf/ino match");
+ }
unlink(xauthfile);
p = strrchr(xauthfile, '/');
if (p != NULL) {
@@ -322,6 +350,7 @@
break;
/* Setup to always have a local .Xauthority. */
+ memset(&xauthfile_sbuf, 0, sizeof(xauthfile_sbuf));
xauthfile = xmalloc(MAXPATHLEN);
strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN);
temporarily_use_uid(pw->pw_uid);
@@ -336,8 +365,20 @@
}
strlcat(xauthfile, "/cookies", MAXPATHLEN);
fd = open(xauthfile, O_RDWR|O_CREAT|O_EXCL, 0600);
- if (fd >= 0)
+ if (fd >= 0) {
+ if (fstat(fd, &xauthfile_sbuf) == -1) {
+ debug("fstat(xauthfile) failed: %s",
+ strerror(errno));
+ memset(&xauthfile_sbuf, 0,
+ sizeof(xauthfile_sbuf));
+ } else {
+ debug("fstat(xauthfile) success: "
+ "dev %lu, ino %lu",
+ xauthfile_sbuf.st_dev,
+ xauthfile_sbuf.st_ino);
+ }
close(fd);
+ }
restore_uid();
fatal_add_cleanup(xauthfile_cleanup_proc, NULL);
success = 1;
@@ -1651,6 +1692,7 @@
xfree(s->auth_data);
return 0;
}
+ memset(&xauthfile_sbuf, 0, sizeof(xauthfile_sbuf));
xauthfile = xmalloc(MAXPATHLEN);
strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN);
temporarily_use_uid(s->pw->pw_uid);
@@ -1667,8 +1709,17 @@
}
strlcat(xauthfile, "/cookies", MAXPATHLEN);
fd = open(xauthfile, O_RDWR|O_CREAT|O_EXCL, 0600);
- if (fd >= 0)
+ if (fd >= 0) {
+ if (fstat(fd, &xauthfile_sbuf) == -1) {
+ debug("fstat(xauthfile) failed: %s",
+ strerror(errno));
+ memset(&xauthfile_sbuf, 0, sizeof(xauthfile_sbuf));
+ } else {
+ debug("fstat(xauthfile) success: dev %lu, ino %lu",
+ xauthfile_sbuf.st_dev, xauthfile_sbuf.st_ino);
+ }
close(fd);
+ }
restore_uid();
fatal_add_cleanup(xauthfile_cleanup_proc, s);
return 1;
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010606143323.G18735>