bug#8292: COREUTILS-8.9: Potential bug in tests/cp/preserve-gid

[gmail]

Le 20/03/2011 15:37, Jim Meyering a écrit :
> gmail wrote:
> ...
> > Normally setuidgid MUST take the parent shell environment, so i
> > finally add a strace in the test to the original source :
> Thanks for investigating.
>
> > --- tests/cp/preserve-gid       2011-03-20 13:34:52.000000000 +0100
> > +++ tests/cp/preserve-gid.orig  2011-01-01 22:19:23.000000000 +0100
> > @@ -38,7 +38,7 @@
> >     u=$1; shift
> >     g=$1; shift
> >     rm -f b || exit 1
> > -  strace -o ../../trace.txt "$@" "$f" b || exit 1
> > +  "$@" "$f" b || exit 1
> >     s=`stat -c '%u %g' b`
> >     if test "x$s" != "x$u $g"; then
> >       # Allow the actual group to match that of the parent directory
> >
> > And got this .... :
> ...
> > > >   execve("/usr/src/coreutils-8.9_build3/src/cp", ["cp", "-p", "c1",
> > "b"], [/* 72 vars */]) = -1 EACCES (Permission denied)
> ...
> > The setuidgid has the PATH, but this is an absolute PATH and PATH
> > traversal with the test uid/gid  fail.
> But why is that?  I.e., it implies that NON_ROOT_USERNAME (your
> "cedric" user) cannot run that program, yet the README instructions
> regarding how to run root tests suggest that NON_ROOT_USERNAME must
> have access to the sources and built programs.
>
> Can you reproduce the failure if you follow these guidelines from
> the README file?
>
>      I find that it is best to unpack and build as a non-privileged
>      user, and then to run the following command as that user in order
>      to run the privilege-requiring tests:
>
>        sudo env PATH="$PATH" NON_ROOT_USERNAME=$USER make -k check-root

it seems i have not been enough precise, to be clear, "cedric" user HAS 
full read/write access to the source and the build :


address@hidden # su cedric

address@hidden find /usr/src/coreutils-8.9 -name preserve-gid

/usr/src/coreutils-8.9/tests/cp/preserve-gid

address@hidden find /usr/src/coreutils-8.9_build -name preserve-gid.log

/usr/src/coreutils-8.9_build/tests/cp/preserve-gid.log

address@hidden touch /usr/src/coreutils-8.9_build3/writing

address@hidden touch /usr/src/coreutils-8.9/writing

address@hidden



And the test fail anyway as root :


    FAIL: cp/preserve-gid (exit: 1)
    ===============================

    ++ initial_cwd_=/usr/src/coreutils-8.9_build3/tests
    ++ fail=0
    +++ testdir_prefix_
    +++ printf gt
    ++ pfx_=gt
    +++ mktempd_ /usr/src/coreutils-8.9_build3/tests gt-preserve-gid.XXXX
    +++ destdir_=/usr/src/coreutils-8.9_build3/tests
    +++ template_=gt-preserve-gid.XXXX
    +++ MAX_TRIES_=4
    ++++ unset TMPDIR
    ++++ mktemp -d -t -p /usr/src/coreutils-8.9_build3/tests 
gt-preserve-gid.XXXX
    +++ d=/usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    +++ test -d /usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    ++++ tr S -
    ++++ ls -dgo /usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    +++ perms=drwx------ 2 4096 Mar 20 17:31 
/usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    +++ test 0 = 0
    +++ echo /usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    +++ return
    ++ test_dir_=/usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    ++ cd /usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    ++ gl_init_sh_nl_=

    ++ IFS=

    +++ expr 1 + 128
    ++ eval 'trap '\''Exit 129'\'' 1'
    +++ trap 'Exit 129' 1
    +++ expr 2 + 128
    ++ eval 'trap '\''Exit 130'\'' 2'
    +++ trap 'Exit 130' 2

    ....

    + t0 c1 1000 1001 setuidgid -g 1000,1001 1000 cp -p
    + f=c1
    + shift
    + u=1000
    + shift
    + g=1001
    + shift
    + rm -f b
    + setuidgid -g 1000,1001 1000 cp -p c1 b
    ++ stat -c '%u %g' b
    + s=1000 1000
    + test 'x1000 1000' '!=' 'x1000 1001'
    + test 'x1000 1000' = 'x1000 0'
    + echo '../../coreutils-8.9/tests/cp/preserve-gid: setuidgid -g 
1000,1001 1000 cp -p c1 b: 1000 1001 != 1000 1000'
    ../../coreutils-8.9/tests/cp/preserve-gid: setuidgid -g 1000,1001 
1000 cp -p c1 b: 1000 1001 != 1000 1000
    + Exit 1
    + set +e
    + exit 1
    + exit 1
    + remove_tmp_
    + __st=1
    + cleanup_
    + :
    + cd /usr/src/coreutils-8.9_build3/tests
    + chmod -R u+rwx 
/usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    + rm -rf /usr/src/coreutils-8.9_build3/tests/gt-preserve-gid.l7ug
    + exit 1

The user '1000', gid '1001' used for the gt-preserve-gid test  no access 
to the /usr/src root, and i see nothing in the guideline requiring to 
give unrestricted source and build access to anyone.
This is under this uid account that the setuigid try to launch the 
builded cp and fail, due to the fact that the PATH variable has a full 
path refering to /usr/src/coreutils-8.9_build3/src, implying a path 
traversal with uid 1000  :

    + 
PATH=/usr/src/coreutils-8.9_build3/src:/usr/src/coreutils-8.9_build3/src:.:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin


This patch is a way to solve  this problem :

--- tests/cp/preserve-gid.orig  2011-01-01 22:19:23.000000000 +0100
+++ tests/cp/preserve-gid.cea   2011-03-20 14:05:19.000000000 +0100
@@ -56,7 +56,8 @@
   f=$1; shift
   u=$1; shift
   g=$1; shift
-  t0 "$f" "$u" "$g" setuidgid -g "$nameless_gid1,$nameless_gid2" \
+  t0 "$f" "$u" "$g" env PATH="../../src:$PATH" \
+      setuidgid -g "$nameless_gid1,$nameless_gid2" \
       "$nameless_uid" "$@"
 }


An other way could be to check if the PATH variable could have a 
relative path instead of a full path, which need far more work.

There could be other problems of this kind in other tests using uid/gid 
change too, but the gt-preserve-gid was the only test that fail in my build.

Regards, Cedric.