Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 4 Feb 1997 16:33:23 -0600 (CST)
From:      Karl Denninger  <karl@Mcs.Net>
To:        jdp@polstra.com (John Polstra)
Cc:        jgreco@solaria.sol.net, gpalmer@freebsd.org, core@freebsd.org, security@freebsd.org
Subject:   Re: 2.1.6+++: crt0.c CRITICAL CHANGE
Message-ID:  <199702042233.QAA02876@Jupiter.Mcs.Net>
In-Reply-To: <199702042148.NAA25064@austin.polstra.com> from "John Polstra" at Feb 4, 97 01:48:04 pm

next in thread | previous in thread | raw e-mail | index | archive | help
> 
> > In revision 1.21 of crt0.c, ache removed these bits of code, and
> > several other sources indicate that removal of the locale code is
> > a sufficient fix.  It therefore seems appropriate to move forward
> > by removing this from crt0.c.
> 
> Nobody seems to dispute that.  But has the actual problem (the buffer
> overflow) been fixed in the locale code?  That needs to be done too.

No.  And BEWARE FOLKS -- "at" calls setlocale, and at runs SUID ROOT.

If the buffer overflow problemS (and yes, I mean plural; just go do a "grep"
for "strcpy" in that directory and you'll see what I mean) aren't all fixed
any call to setlocale() within an SUID program is problematic at BEST.

The entire locale() thing needs to be re-thought for SUID programs.  I'll go
so far given the current spaghetti mess in there as to suggest that
setlocale() be prohibited for anything running with EUID == 0, or EUID !=
RUID until a real review and fix of the entire code set can be conducted.

Yes, that's extreme.  Take a look at it yourself and then tell me its 
unjustified.  For now I'm going through all the SUID executables that
I really need (most have already had their SUIDness revoked here -- shutdown,
for example, doesn't realy need to be SUID since you should be root to run
it anyway) and marking setlocale()s commented out with a #define INSECURE 
until this mess is fixed.

> > If anyone is aware of any undesirable side effects
> 
> The thing to do when you're changing crt0.c is to think very carefully
> about what will happen with all the combinations:
> 
>     new crt0, old libc.so.x.x
>     old crt0, new libc.so.x.x
>     new crt0, new libc.so.x.x
> 
> and test all the combinations too.  I have been burned by this more
> than once, when I had thought I had it all figured out.  It's a
> really unpleasant experience to wake up the morning after a commit
> and find out you've broken make world for a few dozen people.  The
> crt0 changes are particularly insidious, because they can be very
> hard to back out again.

Correct.

> Anyway, I personally don't see such problems in your proposed change.
> 
> PS - Welcome to the development team!
> 
> John P.
> --
>    John Polstra                                       jdp@polstra.com
>    John D. Polstra & Co., Inc.                Seattle, Washington USA
>    "Self-knowledge is always bad news."                 -- John Barth

--
-- 
Karl Denninger (karl@MCS.Net)| MCSNet - The Finest Internet Connectivity
http://www.mcs.net/~karl     | T1's from $600 monthly to FULL DS-3 Service
			     | 99 Analog numbers, 77 ISDN, Web servers $75/mo
Voice: [+1 312 803-MCS1 x219]| Email to "info@mcs.net" WWW: http://www.mcs.net/
Fax:   [+1 773 248-9865]     | 2 FULL DS-3 Internet links; 400Mbps B/W Internal



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199702042233.QAA02876>