svn commit: r199983 - in head: lib/libc/stdlib tools/regression/environ

[Sean C. Farley]

On Tue, 1 Dec 2009, Robert N. M. Watson wrote:

> On 1 Dec 2009, at 11:25, Sean C. Farley wrote:
>
>>> I think it's fair to say that the POSIXization of the environment 
>>> code has been an unmitigated disaster, and speaks to the necessity 
>>> for careful review of those sorts of code changes.
>>
>> As the author of the environment code, I agree that it has been a 
>> painful process.
>>
>> Interestingly, the security issue was a combination of r169661 to 
>> rtld.c, which is a correct action, and the new environ code which was 
>> developed, as opposed to committed, at the same time.  Separately, 
>> the security issue would not have existed.
>
> One immediately pressing question is whether we can mitigate future 
> possible problems along the same lines. The obvious thing is a further 
> (and very careful) audit if all environmental variable use in the base 
> system. But I wonder if there are some other things we could do, such 
> as:
>
> - libc environment scrubbing: try to be more robust in the presence of 
> the unexpected (for example, if you find corrupted stuff, ignore it 
> more robustly); another variation might be to have libc abort(2) if 
> issetugid() and unsetenv(3) would fail.

The preliminary patch I sent earlier should at least make the calls 
behave more like they used to do (go through each variable even if 
corrupt).  However, I do agree that more code (getenv.c and any code 
that calls into it) needs to be verified for more paranoid use of the 
environment.

As for abort(), I was/still am considering having that be the result of 
a corrupt environ array.  If it is corrupt, why attempt to use it? 
unsetenv() may still fail, so it may not abort() for other scenarios.

> - kernel environment scrubbing: the kernel is already responsible for 
> getting those variables across the execve(2) boundary, so is already 
> copying (and to a lesser extent, validating) it, and could learn to be 
> a bit more rigorous in its expectations, perhaps more so for 
> security-sensitive transitions (setuid/setgid/MAC/...)

That is a good point.  I had not thought about kernel validation of the 
environment in addition to the validation performed in libc.

> Brian's changes, although poorly timed, seem like a reasonable 
> direction in this regard: we're stuck with unhelpful APIs, but maybe 
> we can do a better job.

Getting rid of putenv() and especially removing direct access of environ 
(replaced with API call(s)) would be my favorite API changes.

Sean
-- 
scf at FreeBSD.org