[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTi=KyDve19Xu7w1i1kkc7WOSB6U5dzme7rs5tF7T@mail.gmail.com>
Date: Mon, 14 Mar 2011 12:31:18 -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'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.
