Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20090714171811.GA15385@openwall.com>
Date: Tue, 14 Jul 2009 21:18:11 +0400
From: Solar Designer <solar@...nwall.com>
To: oss-security@...ts.openwall.com
Subject: Re: CVE Request (syslog-ng)

I failed to comment on Andreas' message below in time, even though I
intended to, and as a result the Gentoo update for syslog-ng released
this month _might_ be missing a fix for the vulnerabilities identified in
Andreas' message.  Also, the current description for CVE-2008-5110 only
refers to the "chroot jail escape" risk.

The Gentoo advisory does not mention any other issues:

http://lists.openwall.net/bugtraq/2009/07/13/11
http://www.gentoo.org/security/en/glsa/glsa-200907-10.xml

However, the Gentoo "bug":

http://bugs.gentoo.org/show_bug.cgi?id=247278

says:

"Upstream used the latter approach and also solved all the other
concerns raised in the openwall thread."

It refers to the oss-security thread by "the openwall thread", so
apparently the other issues got fixed upstream - but then it is not
clear if the Gentoo fix is an update to newer upstream code (presumably
with all the fixes), a back-port of newer upstream code (maybe with all
the fixes), or a fix for the original chroot escape issue only.

I have not looked at the code.  I am mostly pointing out how the
advisory is not sufficiently verbose (hint!), and I also want to point
out to other distro vendors who might be fixing "this" that there's more
than one issue to fix.  Maybe a second CVE id is needed as well.

Now to address Andreas' message, finally.  The additional real issues
are mentioned closer to the end of the message.  I'll over-quote a bit
to remind us all of the context.

On Tue, Nov 18, 2008 at 09:31:56AM +0100, Andreas Ericsson wrote:
> Solar Designer wrote:
> >>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.

We were talking Unix-like systems, and VMS is not one.  As far as I'm
aware, it does not even have an equivalent of chroot().  As to IRIX, or
any other Unix-like system, I'd be surprised to see the behavior you
describe (chroot(".") failing if the current directory is removed), but
it is possible.

None of this has anything to do with the issue at hand, though, as long
as programs are careful to check return values from syscalls, which they
must do anyway.

> >>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.

I wouldn't say that "those are absolutely safe for chroot-jail usage".
They are not, except for root's.  Chroot'ing to any user-writable dir,
even if all parent dirs are only writable by root, is unsafe, because
library calls invoked by the process might trust files located at
certain "absolute" pathnames, e.g. things such as /etc/nsswitch.conf and
/lib/libnss_*, which suddenly fall under the non-root user's control
(yet the process obviously at least "had" root privs, so attacks by the
user are a concern).

BTW, this is why having an FTP service chroot() users to their home dirs
is an added risk of a root compromise.

> 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).

This is an out of context statement.  It all depends on what risks you'd
like to prevent (possible misconfigurations to detect).  I can think of
risks that are prevented by the above proposed checks and those that are
not.  I don't think it makes sense to discuss this any further, at least
not in this thread, and we'd get to enumerating a lot of possible
misconfigurations (yet we won't get an exhaustive list), and I'm not
interested in that.

My primary point is that there's generally no convention on what
possible misconfigurations a paranoid program should detect, if any.
Yes, I did also mention my opinion on what would be "sane", if any
checking is to be performed at all, but I don't think it requires
further discussion.

Now to the syslog-ng specific stuff, finally:

> >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<--

So this has two additional security issues, besides the missing chdir():

- Not checking return values from setgid(), initgroups(), setuid().

- Possibly not calling initgroups(), thereby inheriting the
supplementary groups from whatever shell started the daemon (this might
include group "root" and the like).  When no target username is known,
but a non-root uid is to be used, then a setgroups() call should be made
to drop the supplementary groups.

I don't know if this was correctly fixed upstream.  I merely read a
statement to this extent in the Gentoo bug.

We're neither using nor redistributing syslog-ng, which is why I don't
care all that much.

Alexander

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.