smbpasswd and euid detection

[Craig Kelley]

On Thu, 2 Jan 2003, Steve Langasek wrote:

> On Thu, Jan 02, 2003 at 01:27:01PM -0700, Craig Kelley wrote:
> > On Thu, 2 Jan 2003, Steve Langasek wrote:
> 
> > > On Thu, Jan 02, 2003 at 10:47:32AM -0700, Craig Kelley wrote:
> > > > For some time now, I've been patching smbpasswd to get rid of the 
> > > > effective UID "detection" that it does.  In 2.2.7a it simply tests if the 
> > > > effective UID differs from the real UID, and if the effective UID is 
> > > > 'root' then it bails:
> 
> > > >    /* Check the effective uid - make sure we are not setuid */
> > > >    if ((geteuid() == (uid_t)0) && (getuid() != (uid_t)0))
> 
> > > > This test will bail out if smbpasswd isn't suid 0, but the process that
> > > > calls it is (eg, a utility agent for changing passwords and such).  I've 
> > > > made a preliminary diff to actually stat() the executable to determine if 
> > > > it is suid 0:
> 
> > > Why does your suid application not either assume full root privileges, or
> > > drop all such privileges, before exec()ing smbpasswd?
> 
> > I've considered that, but thought of it more as treating the symptom 
> > instead of the cause.  A better question may be, why even check for suid?  
> > Why should smbpasswd even care if it's running with effective privileges?  
> > The naive may confuse it with the UNIX passwd program, which is suid root 
> > on some systems, but those with that much knowledge surely understand the 
> > ramifications of giving superuser privileges to an executable.
> 
> I consider confusing smbpasswd with the Unix passwd command a sign that
> one doesn't really have that much knowledge, at least where smbpasswd
> itself is concerned.  It's easy to jump to the conclusion that smbpasswd
> needs root privs to make changes to the smbpasswd file -- it does not --
> and the program has *not* been audited for use as an suid program, so
> it's dangerous to treat it the same as passwd.
> 
> So if someone can run smbpasswd indirectly from an suid wrapper, there's
> still a high potential for security problems, the same as if smbpasswd is
> suid itself.  If you need to let users call smbpasswd in an suid root
> context, your wrapper should do its own vetting of the user input and
> then assume full root privileges.

Then let's add suid checking to every program.

They can all be abused, and the same argument should apply.

Regardless, the patch I presented actually does what the the warning 
message claims it's doing.  It stat()'s the actual binary of smbpasswd to 
see if it's suid or not.  It doesn't add any dependencies, and it should 
work on all systems capable of handling geteuid(), which smbpasswd already 
uses.

-- 
Craig Kelley  -- kellcrai at isu.edu
Turn In Your Neighbor Today!  http://www.bsa.org/usa/report/report.php
http://www.isu.edu/~kellcrai finger ink at inconnu.isu.edu for PGP block