[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTikRksmX+y=8zgXzJAfq2mCQHCFhtU3saXEVt+Za@mail.gmail.com>
Date: Tue, 15 Mar 2011 09:13:00 -0400
From: Dan Rosenberg <dan.j.rosenberg@...il.com>
To: oss-security@...ts.openwall.com
Cc: Ludwig Nussel <ludwig.nussel@...e.de>, Petr Baudis <pasky@...e.cz>
Subject: Re: Suid mount helpers fail to anticipate RLIMIT_FSIZE
I did a survey of some suid helpers I'm aware of. Here's the existing behavior:
util-linux mount
=============
* Edits /etc/mtab.tmp with custom my_addmntent(), behaves identically
to glibc addmntent() in terms of return code
* Succeeds on partial writes, does not remove temp file on failure
(could result in additional corruption of /etc/mtab through multiple
invocations), does not remove lock file /etc/mtab~ on failure (also an
issue)
fusermount (FUSE)
================
* Does not edit mtab directly, calls into util-linux mount/umount, no
changes needed
mount.cifs (samba)
================
* mount.cifs edits /etc/mtab directly, no cleanup on addmntent() failure
* umount.cifs edits /etc/mtab.tmp but does not check return code of addmntent()
ncpmount (ncpfs)
==============
* ncpmount edits /etc/mtab directly, no cleanup on failure, does not
remove lock file /etc/mtab~ on failure
* ncpumount edits /etc/mtab.tmp but does not check return code of addmntent()
vmware-hgfsmounter (open-vm-tools)
===============================
* edits /etc/mtab directly, no cleanup on failure
Regards,
Dan
On Mon, Mar 14, 2011 at 12:31 PM, Dan Rosenberg
<dan.j.rosenberg@...il.com> wrote:
> I've done some further investigation, and have found one of the
> underlying problems. addmntent() will return 0 (success) even if the
> write was truncated:
>
> return (fprintf (stream, "%s %s %s %s %d %d\n",
> mntcopy.mnt_fsname,
> mntcopy.mnt_dir,
> mntcopy.mnt_type,
> mntcopy.mnt_opts,
> mntcopy.mnt_freq,
> mntcopy.mnt_passno)
> < 0 ? 1 : 0);
>
> Of course, this only matters if the process is catching the SIGXFSZ
> that gets thrown if the resource limit is exceeded, but nearly all
> suid mount helpers block or ignore signals (if they don't, that's an
> additional problem, because the process could be terminated mid-write,
> corrupting /etc/mtab or leaving a stale lockfile, for example).
>
> So, I think the first step is to patch glibc to return success in
> these functions if and only if the *full* contents have been written.
> Then, it will be possible to have proper error handling in these
> helper utilities. Currently, there's really no way for these programs
> to know whether or not their calls to addmntent() actually succeeded
> besides installing a special signal handler for SIGXFSZ (ugly).
>
> After some further thinking and discussion, I think what needs to be
> done is ensuring that all helpers make mtab edits to a temporary file,
> and have proper error handling that cleans up correctly without
> copying over to the actual /etc/mtab if anything bad happens.
> Currently, some mount helpers edit /etc/mtab directly, and others use
> a temporary file but don't have the proper error handling.
>
> I think this one's going to fall into the hands of package maintainers
> and distros, I don't have time to fix all of these.
>
> -Dan
>
> On Mon, Mar 14, 2011 at 8:32 AM, Dan Rosenberg
> <dan.j.rosenberg@...il.com> wrote:
>> Sigh. Unfortunately I think this is the truth - I just wish there
>> were an easier way of addressing this besides patching every affected
>> helper individually. Unless anyone else has any ideas, I'll write up
>> some patches for affected programs later today.
>>
>> -Dan
>>
>> On Mon, Mar 14, 2011 at 8:14 AM, Ludwig Nussel <ludwig.nussel@...e.de> wrote:
>>> Dan Rosenberg wrote:
>>>> There are a few possible options We could patch glibc to try to
>>>> raise the rlimit in addmntent(). [...]
>>>
>>> Citing our glibc maintainer Petr Baudis via Bugzilla:
>>>
>>> | I have been thinking about it and I'm not at all sure the proposed solution
>>> | makes sense. First, this may also concern the obscure interfaces like
>>> | putspent() (not sure if anyone uses these, moreover in security relevant
>>> | contexts). Second, messing with RLIMIT_FSIZE within library routine is just
>>> | evil. The caller may be multi-threaded or just do something else between
>>> | setpwent() and endpwent() too and RLIMIT_FSIZE is just evil. All setuid
>>> | programs must sanitize things like this, on their own terms.
>>>
>>> cu
>>> Ludwig
>>>
>>> --
>>> (o_ Ludwig Nussel
>>> //\
>>> V_/_ http://www.suse.de/
>>> SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
>>>
>>
>
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.
