[12086] in bugtraq
Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy]
daemon@ATHENA.MIT.EDU (Jeff Long)
Fri Oct 1 14:26:21 1999
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="------------EE7E52CF326C0771F6E0B66B"
Message-Id: <37F3E326.8A5407A8@kestrel.cc.ukans.edu>
Date: Thu, 30 Sep 1999 17:24:38 -0500
Reply-To: Jeff Long <long@KESTREL.CC.UKANS.EDU>
From: Jeff Long <long@KESTREL.CC.UKANS.EDU>
X-To: BUGTRAQ@SECURITYFOCUS.COM
To: BUGTRAQ@SECURITYFOCUS.COM
This is a multi-part message in MIME format.
--------------EE7E52CF326C0771F6E0B66B
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Dan Astoorian wrote:
>
> On Wed, 29 Sep 1999 16:59:48 EDT, Sylvain Robitaille writes:
> > I don't promise the most impressive code, but it has been tested (on
> > Digital Unix) and I believe it works correctly. Comments are of course
> > welcome...
>
> I have a couple of serious concerns about this patch.
>
> 1) It leaves behind a race condition; a symlink created between the
> lstat() and the bind() will still get happily followed. The race
> condition could be minimized by moving the lstat() and the bind()
> closer together, but it can't be eliminated this way. This is why
> it's important for the check to be made in the kernel, where it can
> be done atomically.
<snip>
> The race condition is a hard problem; if bind() follows symlinks, it is
> *impossible* to safely use it in a directory writable by anyone other
> than geteuid().
>
> I haven't looked into what would be involved in creating a proper patch,
> but appropriate ways to fix the problem *might* include:
>
> - changing the process's effective userid/groupid/credentials to match
> the target user before doing the bind(), so that directories not
> writable by the user are also not writable by the code doing the
> bind(); or
<snip>
Seeing the race problems with the previous two patches I thought I would
take a shot at one. It changes the effective uid/gid to the user
logging in before doing the bind() (and then resets them after) which
seems to take care of the problem. It also chown's the
/tmp/ssh-<username> directory before doing the bind in the case that it
did not already exist so that the user owns it before performing the
bind. On Digital Unix 4.0D this seems to take care of the problem. The
bind() will fail if a symlink exists to a file that the user would
normally not be able to write to (such as /etc/nologin). The only other
difference after logging in is that the socket will now be owned by the
user instead of root but I can't think of a reason why this would be
bad.
If anyone sees problems in this patch please let me know.
Jeff Long
--------------EE7E52CF326C0771F6E0B66B
Content-Type: text/plain; charset=us-ascii;
name="newchannels.c.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="newchannels.c.patch"
*** newchannels.c.original Thu Sep 30 16:58:22 1999
--- newchannels.c Thu Sep 30 17:13:24 1999
***************
*** 2262,2267 ****
--- 2262,2269 ----
struct stat st, st2, parent_st;
mode_t old_umask;
char *last_dir;
+ uid_t saved_euid = 0;
+ gid_t saved_egid = 0;
if (auth_get_socket_name() != NULL)
fatal("Protocol error: authentication forwarding requested twice.");
***************
*** 2411,2427 ****
creating unix-domain sockets, you might not be able to use
ssh-agent connections on your system */
old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
!
! if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0)
! packet_disconnect("Agent socket bind failed: %.100s", strerror(errno));
!
! umask(old_umask);
!
if (directory_created)
if (chown(".", pw->pw_uid, pw->pw_gid) < 0)
packet_disconnect("Agent socket directory chown failed: %.100s",
strerror(errno));
/* Start listening on the socket. */
if (listen(sock, 5) < 0)
packet_disconnect("Agent socket listen failed: %.100s", strerror(errno));
--- 2413,2442 ----
creating unix-domain sockets, you might not be able to use
ssh-agent connections on your system */
old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
!
if (directory_created)
if (chown(".", pw->pw_uid, pw->pw_gid) < 0)
packet_disconnect("Agent socket directory chown failed: %.100s",
strerror(errno));
+ saved_euid = geteuid();
+ saved_egid = getegid();
+
+ if (setegid(pw->pw_gid) < 0)
+ packet_disconnect("Agent socket setegid failed: %.100s", strerror(errno));
+ if (seteuid(pw->pw_uid) < 0)
+ packet_disconnect("Agent socket seteuid failed: %.100s", strerror(errno));
+
+ if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0)
+ packet_disconnect("Agent socket bind failed: %.100s", strerror(errno));
+
+ if (seteuid(saved_euid) < 0)
+ packet_disconnect("Agent socket re-seteuid failed: %.100s", strerror(errno));
+ if (setegid(saved_egid) < 0)
+ packet_disconnect("Agent socket re-setegid failed: %.100s", strerror(errno));
+
+ umask(old_umask);
+
/* Start listening on the socket. */
if (listen(sock, 5) < 0)
packet_disconnect("Agent socket listen failed: %.100s", strerror(errno));
--------------EE7E52CF326C0771F6E0B66B--
