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

From: gmail <[email-masked]>
Date: Sat, 19 Mar 2011 17:21:59 +0100

Hi all,

This is my first contact and mail with this mailing list, to summarize 
i'm a coder maintening a personnal home server using several GNU tools 
since the late 90's.

I build COREUTILS-8.9 in a chroot jail (gcc 4.5.2/libc 2.13/binutils 
2.21/make 3.82) with an athlon architecture on ext3 FS and, as root,  
got one failed test on  tests/cp/preserve-gid :

     [root@pompomgalli] mkdir coreutils-8.9_build && cd coreutils-8.9_build

     [root@pompomgalli] ../coreutils-8.9/configure && make

     ...

     [root@pompomgalli] NON_ROOT_USERNAME=cedric make -k check 
TESTS=cp/preserve-gid VERBOSE=yes

     [root@pompomgalli] less tests/cp/preserve-gid.log

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

     ++ initial_cwd_=/usr/src/coreutils-8.9_build/tests
     ++ fail=0
     +++ testdir_prefix_
     +++ printf gt
     ++ pfx_=gt
     +++ mktempd_ /usr/src/coreutils-8.9_build/tests gt-preserve-gid.XXXX
     +++ destdir_=/usr/src/coreutils-8.9_build/tests
     +++ template_=gt-preserve-gid.XXXX
     +++ MAX_TRIES_=4
     ++++ unset TMPDIR
     ++++ mktemp -d -t -p /usr/src/coreutils-8.9_build/tests 
gt-preserve-gid.XXXX
     +++ d=/usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     +++ test -d /usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     ++++ tr S -
     ++++ ls -dgo /usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     +++ perms=drwx------ 2 4096 Mar 19 15:18 
/usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     +++ test 0 = 0
     +++ echo /usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     +++ return
     ++ test_dir_=/usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     ++ cd /usr/src/coreutils-8.9_build/tests/gt-preserve-gid.jLiF
     ++ 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
     +++ expr 3 + 128
     ++ eval 'trap '\''Exit 131'\'' 3'
     +++ trap 'Exit 131' 3
     +++ expr 13 + 128
     ++ eval 'trap '\''Exit 141'\'' 13'
     +++ trap 'Exit 141' 13
     +++ expr 15 + 128
     ++ eval 'trap '\''Exit 143'\'' 15'
     +++ trap 'Exit 143' 15
     ++ trap remove_tmp_ 0
     + path_prepend_ ../src
     + test 1 '!=' 0
     + path_dir_=../src
     ++ cd /usr/src/coreutils-8.9_build/tests/../src
     ++ echo /usr/src/coreutils-8.9_build/src
     + abs_path_dir_=/usr/src/coreutils-8.9_build/src
     + 
PATH=/usr/src/coreutils-8.9_build/src:/usr/src/coreutils-8.9_build/src:.:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin
     + create_exe_shims_ /usr/src/coreutils-8.9_build/src
     + return 0
     + shift
     + test 0 '!=' 0
     + export PATH
     + print_ver_ cp
     + test yes = yes
     + local i
     + env cp --version
     cp (GNU coreutils) 8.9
     Copyright (C) 2011 Free Software Foundation, Inc.
     License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>.
     This is free software: you are free to change and redistribute it.
     There is NO WARRANTY, to the extent permitted by law.
     [Written by Torbj"orn Granlund, David MacKenzie, and Jim Meyering.
     + require_root_
     + uid_is_privileged_
     ++ id -u
     + my_uid=0
     + NON_ROOT_USERNAME=cedric
     ++ id -g cedric
     + NON_ROOT_GROUP=2003
     + working_umask_or_skip_
     + umask 022
     + touch file1 file2
     + chmod 644 file2
     ++ uniq
     ++ sed 's/ .*//'
     ++ ls -l file1 file2
     + perms=-rw-r--r--
     + rm -f file1 file2
     ++ id -g
     + primary_group_num=0
     ++ perl -le '
       foreach my $i (1000..16*1024-1)
         {
           getpwuid $i or (print $i), exit
         }
     '
     + nameless_uid=1000
     ++ perl -le '
       foreach my $i (1000..16*1024)
         {
           getgrgid $i or (print $i), exit
         }
     '
     + nameless_gid1=1000
     ++ perl -le '
       foreach my $i (1000+1..16*1024)
         {
           getgrgid $i or (print $i), exit
         }
     '
     + nameless_gid2=1001
     + test -z 1000
     + test -z 1000
     + test -z 1001
     + chown +1000:+0 .
     + create a0 0 0
     + echo a0
     + chown +0:+0 a0
     + create b0 1000 1000
     + echo b0
     + chown +1000:+1000 b0
     + create b1 1000 1001
     + echo b1
     + chown +1000:+1001 b1
     + create c0 0 1000
     + echo c0
     + chown +0:+1000 c0
     + create c1 0 1001
     + echo c1
     + chown +0:+1001 c1
     + t0 a0 0 0 cp
     + f=a0
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp a0 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 b0 0 0 cp
     + f=b0
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp b0 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 b1 0 0 cp
     + f=b1
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp b1 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 c0 0 0 cp
     + f=c0
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp c0 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 c1 0 0 cp
     + f=c1
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp c1 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 a0 0 0 cp -p
     + f=a0
     + shift
     + u=0
     + shift
     + g=0
     + shift
     + rm -f b
     + cp -p a0 b
     ++ stat -c '%u %g' b
     + s=0 0
     + test 'x0 0' '!=' 'x0 0'
     + t0 b0 1000 1000 cp -p
     + f=b0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + cp -p b0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t0 b1 1000 1001 cp -p
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1001
     + shift
     + rm -f b
     + cp -p b1 b
     ++ stat -c '%u %g' b
     + s=1000 1001
     + test 'x1000 1001' '!=' 'x1000 1001'
     + t0 c0 0 1000 cp -p
     + f=c0
     + shift
     + u=0
     + shift
     + g=1000
     + shift
     + rm -f b
     + cp -p c0 b
     ++ stat -c '%u %g' b
     + s=0 1000
     + test 'x0 1000' '!=' 'x0 1000'
     + t0 c1 0 1001 cp -p
     + f=c1
     + shift
     + u=0
     + shift
     + g=1001
     + shift
     + rm -f b
     + cp -p c1 b
     ++ stat -c '%u %g' b
     + s=0 1001
     + test 'x0 1001' '!=' 'x0 1001'
     + t1 a0 1000 1000 cp
     + f=a0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 a0 1000 1000 setuidgid -g 1000,1001 1000 cp
     + f=a0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp a0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 b0 1000 1000 cp
     + f=b0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 b0 1000 1000 setuidgid -g 1000,1001 1000 cp
     + f=b0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp b0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 b1 1000 1000 cp
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 b1 1000 1000 setuidgid -g 1000,1001 1000 cp
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp b1 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 c0 1000 1000 cp
     + f=c0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 c0 1000 1000 setuidgid -g 1000,1001 1000 cp
     + f=c0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp c0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 c1 1000 1000 cp
     + f=c1
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 c1 1000 1000 setuidgid -g 1000,1001 1000 cp
     + f=c1
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp c1 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 a0 1000 1000 cp -p
     + f=a0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 a0 1000 1000 setuidgid -g 1000,1001 1000 cp -p
     + f=a0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp -p a0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 b0 1000 1000 cp -p
     + f=b0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 b0 1000 1000 setuidgid -g 1000,1001 1000 cp -p
     + f=b0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp -p b0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 b1 1000 1001 cp -p
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1001
     + shift
     + t0 b1 1000 1001 setuidgid -g 1000,1001 1000 cp -p
     + f=b1
     + shift
     + u=1000
     + shift
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1001
     + shift
     + t0 b1 1000 1001 setuidgid -g 1000,1001 1000 cp -p
     + f=b1
     + shift
     + u=1000
     + shift
     + g=1001
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp -p b1 b
     ++ stat -c '%u %g' b
     + s=1000 1001
     + test 'x1000 1001' '!=' 'x1000 1001'
     + t1 c0 1000 1000 cp -p
     + f=c0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + t0 c0 1000 1000 setuidgid -g 1000,1001 1000 cp -p
     + f=c0
     + shift
     + u=1000
     + shift
     + g=1000
     + shift
     + rm -f b
     + setuidgid -g 1000,1001 1000 cp -p c0 b
     ++ stat -c '%u %g' b
     + s=1000 1000
     + test 'x1000 1000' '!=' 'x1000 1000'
     + t1 c1 1000 1001 cp -p
     + f=c1
     + shift
     + u=1000
     + shift
     + g=1001
     + shift
     + 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_

To investigate, i disable  the remove_tmp_ function and take a look at 
the temporary folder :

     [root@pompomgalli] cd tests/gt-preserve-gid.jLiF/

     [root@pompomgalli] ls -ld *

     -rw-r--r--    1 root     root            3 Mar 19 15:18 a0
     -rw-r--r--    1 1000     1000            3 Mar 19 15:18 b
     -rw-r--r--    1 1000     1000            3 Mar 19 15:18 b0
     -rw-r--r--    1 1000     1001            3 Mar 19 15:18 b1
     -rw-r--r--    1 root     1000            3 Mar 19 15:18 c0
     -rw-r--r--    1 root     1001            3 Mar 19 15:18 c1

     [root@pompomgalli] ../../src/setuidgid -g 1000,1001 1000 
../../src/cp -p c1 ww

     [root@pompomgalli] ls -ld c1 ww

     -rw-r--r--    1 root     1001            3 Mar 19 15:18 c1
     -rw-r--r--    1 1000     1001            3 Mar 19 15:18 ww

The gid has been correctly preserved, so this is an environment issue :

     [root@pompomgalli] ../../src/setuidgid -g 1000,1001 1000 cp -p c1 ww2

     -rw-r--r--    1 root     1001            3 Mar 19 15:18 c1
     -rw-r--r--    1 1000     1001            3 Mar 19 15:18 ww
     -rw-r--r--    1 1000     1000            3 Mar 19 15:18 ww2

The PATH variables was :

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

But the cp call ( "$@" "$f" b  ) in preserve-gid give :

     setuidgid -g 1000,1001 1000 cp -p c1 b

So the setuidgid used is the /usr/src/coreutils-8.9_build/src/setuidgid 
command, but, as the PATH variable isn't an exported variable, the 
setuigid command look for /bin/cp instead of 
/usr/src/coreutils-8.9_build/src/cp.
The local /bin/cp command used for the build was not intented to support 
that kind of operation, and i guess the test should concern the 
coreutils's cp command in the coreutils's build tree ^^.

Consequently, i make and apply this patch :

--- coreutils-8.9/tests/cp/preserve-gid.orig    2011-01-01 
22:19:23.000000000 +0100
+++ coreutils-8.9/tests/cp/preserve-gid 2011-03-19 16:04:59.000000000 +0100
@@ -93,28 +93,28 @@
  create c0 0 "$nameless_gid1"
  create c1 0 "$nameless_gid2"

-t0 a0 0 0 cp
-t0 b0 0 0 cp
-t0 b1 0 0 cp
-t0 c0 0 0 cp
-t0 c1 0 0 cp
+t0 a0 0 0 "${abs_path_dir_}/cp"
+t0 b0 0 0 "${abs_path_dir_}/cp"
+t0 b1 0 0 "${abs_path_dir_}/cp"
+t0 c0 0 0 "${abs_path_dir_}/cp"
+t0 c1 0 0 "${abs_path_dir_}/cp"

-t0 a0 0 0 cp -p
-t0 b0 "$nameless_uid" "$nameless_gid1" cp -p
-t0 b1 "$nameless_uid" "$nameless_gid2" cp -p
-t0 c0 0 "$nameless_gid1" cp -p
-t0 c1 0 "$nameless_gid2" cp -p
+t0 a0 0 0 "${abs_path_dir_}/cp" -p
+t0 b0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp" -p
+t0 b1 "$nameless_uid" "$nameless_gid2" "${abs_path_dir_}/cp" -p
+t0 c0 0 "$nameless_gid1" "${abs_path_dir_}/cp" -p
+t0 c1 0 "$nameless_gid2" "${abs_path_dir_}/cp" -p

-t1 a0 "$nameless_uid" "$nameless_gid1" cp
-t1 b0 "$nameless_uid" "$nameless_gid1" cp
-t1 b1 "$nameless_uid" "$nameless_gid1" cp
-t1 c0 "$nameless_uid" "$nameless_gid1" cp
-t1 c1 "$nameless_uid" "$nameless_gid1" cp
+t1 a0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp"
+t1 b0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp"
+t1 b1 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp"
+t1 c0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp"
+t1 c1 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp"

-t1 a0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 b0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 b1 "$nameless_uid" "$nameless_gid2" cp -p
-t1 c0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 c1 "$nameless_uid" "$nameless_gid2" cp -p
+t1 a0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp" -p
+t1 b0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp" -p
+t1 b1 "$nameless_uid" "$nameless_gid2" "${abs_path_dir_}/cp" -p
+t1 c0 "$nameless_uid" "$nameless_gid1" "${abs_path_dir_}/cp" -p
+t1 c1 "$nameless_uid" "$nameless_gid2" "${abs_path_dir_}/cp" -p

  Exit $fail

But absolute path imply path traversal which can fail due to 
permissions, so here is the patch i my final choice :

--- coreutils-8.9/tests/cp/preserve-gid.orig    2011-01-01 
22:19:23.000000000 +0100
+++ coreutils-8.9/tests/cp/preserve-gid 2011-03-19 16:37:11.000000000 +0100
@@ -93,28 +93,28 @@
  create c0 0 "$nameless_gid1"
  create c1 0 "$nameless_gid2"

-t0 a0 0 0 cp
-t0 b0 0 0 cp
-t0 b1 0 0 cp
-t0 c0 0 0 cp
-t0 c1 0 0 cp
+t0 a0 0 0 "../${path_dir_}/cp"
+t0 b0 0 0 "../${path_dir_}/cp"
+t0 b1 0 0 "../${path_dir_}/cp"
+t0 c0 0 0 "../${path_dir_}/cp"
+t0 c1 0 0 "../${path_dir_}/cp"

-t0 a0 0 0 cp -p
-t0 b0 "$nameless_uid" "$nameless_gid1" cp -p
-t0 b1 "$nameless_uid" "$nameless_gid2" cp -p
-t0 c0 0 "$nameless_gid1" cp -p
-t0 c1 0 "$nameless_gid2" cp -p
+t0 a0 0 0 "../${path_dir_}/cp" -p
+t0 b0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp" -p
+t0 b1 "$nameless_uid" "$nameless_gid2" "../${path_dir_}/cp" -p
+t0 c0 0 "$nameless_gid1" "../${path_dir_}/cp" -p
+t0 c1 0 "$nameless_gid2" "../${path_dir_}/cp" -p

-t1 a0 "$nameless_uid" "$nameless_gid1" cp
-t1 b0 "$nameless_uid" "$nameless_gid1" cp
-t1 b1 "$nameless_uid" "$nameless_gid1" cp
-t1 c0 "$nameless_uid" "$nameless_gid1" cp
-t1 c1 "$nameless_uid" "$nameless_gid1" cp
+t1 a0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp"
+t1 b0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp"
+t1 b1 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp"
+t1 c0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp"
+t1 c1 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp"

-t1 a0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 b0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 b1 "$nameless_uid" "$nameless_gid2" cp -p
-t1 c0 "$nameless_uid" "$nameless_gid1" cp -p
-t1 c1 "$nameless_uid" "$nameless_gid2" cp -p
+t1 a0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp" -p
+t1 b0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp" -p
+t1 b1 "$nameless_uid" "$nameless_gid2" "../${path_dir_}/cp" -p
+t1 c0 "$nameless_uid" "$nameless_gid1" "../${path_dir_}/cp" -p
+t1 c1 "$nameless_uid" "$nameless_gid2" "../${path_dir_}/cp" -p

  Exit $fail

The checks are now all succesfull on my platform.

     ...
     =======================
     All 213 tests passed
     (16 tests were not run)
     =======================
     make[6]: Leaving directory `/usr/src/coreutils-8.9_build/gnulib-tests'
     make[5]: Leaving directory `/usr/src/coreutils-8.9_build/gnulib-tests'
     make[4]: Leaving directory `/usr/src/coreutils-8.9_build/gnulib-tests'
     make[3]: Leaving directory `/usr/src/coreutils-8.9_build/gnulib-tests'
     make[2]: Leaving directory `/usr/src/coreutils-8.9_build/gnulib-tests'
     make[2]: Entering directory `/usr/src/coreutils-8.9_build'
     make[2]: Nothing to be done for `check-am'.
     make[2]: Leaving directory `/usr/src/coreutils-8.9_build'
     make[1]: Leaving directory `/usr/src/coreutils-8.9_build'

The cp call ( "$@" "$f" b  ) in preserve-gid with arguments with quote 
could be a problem for some old fahsioned shells, even if i don't see 
any concerned shells.

Some commands in other test scripts could have the same problem and 
should be then enforced in the same way, i'll do this if this is the 
right solution.

Regards, Cedric.