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. 2) Using popen() within a privileged process is somewhat reckless; it potentially opens up the usual risks of shell-mischief, although I haven't gone digging for any specific holes you've opened up. What's wrong with using syslog? 3) This isn't a vulnerability, but as a matter of principle, I don't trust any code that could wind up containing this line: + if (dirname[strlen(dirname)] == '/') dirname[strlen(dirname)] = 0; (Trust me: dirname[strlen(dirname)] != '/'. Presumably a "- 1" was intended someplace or two?) 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 - using a different location for the Unix domain socket--one which is verifiably manipulable only by root. [As long as I'm here: it's been pointed out to me that my test program was missing a semicolon after "close(fd)". This was, of course, a cut-and-paste error; my apologies.] Cheers, -- People shouldn't think that it's better to have Dan Astoorian loved and lost than never loved at all. It's Sysadmin, CS Lab not, it's better to have loved and won. All djastat_private the other options really suck. --Dan Redican
This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 15:06:11 PDT