Bugtraq mailing list archives
Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy]
From: long () KESTREL CC UKANS EDU (Jeff Long)
Date: Thu, 30 Sep 1999 17:28:38 -0500
Oh crud, this wasn't tested on Digital Unix 4.0D. It was tested before and after on Compaq Tru64 5.0. Jeff Long Jeff Long wrote:
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
*** 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));
Current thread:
- Re: Fix for ssh-1.2.27 symlink/bind problem, (continued)
- Re: Fix for ssh-1.2.27 symlink/bind problem Olaf Seibert (Oct 04)
- Re: Fix for ssh-1.2.27 symlink/bind problem Dan Astoorian (Oct 05)
- Weakness In "The Matrix" Screensaver For Windows Boyce, Nick (Oct 04)
- Re: Weakness In "The Matrix" Screensaver For Windows Glenn Walker (Oct 05)
- Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] Chris Keane (Oct 01)
- Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] Sylvain Robitaille (Oct 04)
- Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] Dan Astoorian (Oct 04)
- FireWall-1 weakness? Rosner, D (Oct 04)
- WIn98 port security query Jay R. Ashworth (Oct 01)