[bug #20534] Security: cvs client too keen to change permissions

[Ed Avis]

URL:
  <http://savannah.nongnu.org/bugs/?20534>

                 Summary: Security: cvs client too keen to change permissions
                 Project: Concurrent Versions System
            Submitted by: epaepa
            Submitted on: Thursday 19/07/07 at 16:02
                Category: Bug Report
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
                 Release: 
           Fixed Release: None
   Fixed Feature Release: None

    _______________________________________________________

Details:

It seems that 'cvs update', when creating a new file in the working copy,
copies permissions from the file in the repository to the local checked-out
file.  That is probably fine for the read, write and execute bits, but it's
dangerous to copy the suid or sgid flags.  This is a potential security hole
- if an attacker can persuade you to connect to a CVS repository he controls
and do a cvs update, then he can trick you into creating a suid binary owned
by you.

To reproduce:

# First the user makes an ordinary CVS project and starts work.
% export CVSROOT=$HOME/cvsroot
% mkdir $CVSROOT
% cvs init
% mkdir ~/scratch
% cd ~/scratch
% cvs import -mmsg test a b
% cvs co test
% cd test
% echo '#!/bin/sh' >exe
% echo 'echo hello' >>exe
% chmod a+x exe
% cvs add exe
% cvs commit -mmsg exe

# Now let us suppose we are the attacker and we modify the
# permissions of exe,v in the repository.  Although not shown
# here, the exploit also works if the repository is owned by
# some other user, as long as the checking-out user has
# permission to create cvs lock files.
% cd $CVSROOT/test
% chmod u+s exe,v

# Now back to the ordinary user, who for whatever reason decides
# to remove his working copy of exe and check it out fresh.
% cd ~/scratch/test
% rm exe
% cvs update exe
% ls -l exe

# Expected result: cvs was sensible enough not to copy the suid
# bit, so the permissions will be rwxr-xr-x or similar.
#
# Actual result: permissions shown as -rwsr-xr-x

I'd suggest that the only permission bit that should be set in the working
copy by cvs is the user execute bit.  The other permission bits are too
dangerous to set without user intervention.  Especially don't set the suid or
sgid bits.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?20534>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/