oss-sec mailing list archives
Re: CVE Request (syslog-ng)
From: Andreas Ericsson <ae () op5 se>
Date: Tue, 18 Nov 2008 09:31:56 +0100
Solar Designer wrote:
On Mon, Nov 17, 2008 at 11:06:53PM +0100, Andreas Ericsson wrote:The correct sequence is: chdir(jail_path); chroot(".");That's a correct sequence, but not the only correct one.
Well, "chroot(jail_path); chdir("/");" does effectively the same thing (ie, avoid re-using jail_path), so they're equally race- resistant, ofcourse.
The chroot() call will fail if the directory no longer exists,No. I'm not sure if this is required by any standard, by the behavior I'm used to seeing is that the current directory for a process will sort of continue to exist for that process despite of being removed.
This doesn't hold true on VMS and some Irix releases. Now that you say it, I haven't run into it on more recent systems, but I remember it happening on those about two years back at a customers site. HP dev-center used to have an (Open)VMS system, but I can't seem to find it now, or I would verify it there.
Paranoid programs only accept absolute non-symlink paths to the jail and issue getcwd() after having entered it to make sure they ended up in the proper directory.I find this ridiculous - a program can't possibly defend itself against every kind of unsafe use by the sysadmin. If a program chooses to do any sanity checking on the provided pathname, then a sane check would be to ensure that all directories in the path are only writable by root.
That would fail with Owl's setup of per-user /tmp/.private/$USER dirs, even if those are absolutely safe for chroot-jail usage otherwise. Assuming no symlinks, it's enough to ensure that "." and ".." are root- owned and 0700 once the directory has been entered, and quite a lot simpler. This has been verified by at least eight separate security researchers in severely paranoid industries (weapons research among others).
Besides the missing chdir(), another very common issue with "privilege dropping" programs is forgetting to drop or re-initialize supplementary groups. Not checking return values from any of the functions involved is about as common... (I have not checked syslog-ng for any of this.)
It's not stellar in that respect. Here's the effective code, printable with "sed -n 275,297p src/main.c" from the latest syslog-ng snapshot. --8<--8<--8<-- int setup_creds(void) { if (chroot_dir) { if (chroot(chroot_dir) < 0) { msg_error("Error during chroot()", evt_tag_errno(EVT_TAG_OSERROR, errno), NULL); return 0; } } if (uid || gid || run_as_user) { setgid(gid); if (run_as_user) initgroups(run_as_user, gid); setuid(uid); } return 1; } --8<--8<--8<-- -- Andreas Ericsson andreas.ericsson () op5 se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231
Current thread:
- CVE Request (syslog-ng) Josh Bressers (Nov 17)
- Re: CVE Request (syslog-ng) Steven M. Christey (Nov 17)
- Re: CVE Request (syslog-ng) Andreas Ericsson (Nov 17)
- Re: CVE Request (syslog-ng) Solar Designer (Nov 17)
- Re: CVE Request (syslog-ng) Andreas Ericsson (Nov 18)
- Re: CVE Request (syslog-ng) Andreas Ericsson (Nov 17)
- Re: CVE Request (syslog-ng) Steven M. Christey (Nov 17)