Hello Ingo, 

schwa...@usta.de (Ingo Schwarze), 2010.12.30 (Thu) 00:08 (CET):
> I consider this a bug in security(8).
> 
> The following is the best i could come up with so far; make sure
> to wear your sed-peril-proof sunglasses before reading the patch.
 
Would have taken me ages to come up with that.

> This still mangles the file name, but at least you have a chance
> to find it on your disk.  Anybody has a better plan?
 
Solve the problem ;-)

To me it looks like the core of the problem is ls(1) having spaces as
field delimiter. 

Clean solution would be to use null byte throughout the chained
commands, far beyond what I could come up with patches for:
- A ``-0'' switch to ls(1). Would have a different meaning than in
  xargs(1) and find(1) (field vs. record delimiter). 
- Teach sort(1) -t switch to take nul as delimiter
- Teach join(1) -t switch to take nul as delimiter

Or what I can do:
- use pipe as delimiter
- downside: it changes the format of 
  CUR=/var/backups/device.current
  BACK=/var/backups/device.backup
  which currently is just the output of ``ls -ldgT'' and cannot be
  easily restored after the suggested transformations.

--- security.orig       Thu Dec 30 15:09:55 2010
+++ security    Thu Dec 30 15:33:55 2010
@@ -427,14 +427,17 @@
        \) -a -prune -o \
        -type f -a \( -perm -u+s -o -perm -g+s \) -print0 -o \
        ! -type d -a ! -type f -a ! -type l -a ! -type s -a ! -type p \
-       -print0 | xargs -0 -r ls -ldgT | sort +9 > $LIST
+       -print0 | xargs -0 -r ls -ldgT | tr ' ' '|' | \
+       sed 'h;s,[^/]*,,;s,|, ,g;x;s,/.*,,;G;s/\n//' | \
+       tr -s '|' | sort -t '|' +9 > $LIST
 )
 
 # Display any changes in the setuid/setgid file list.
 next_part "Checking setuid/setgid files and devices:"
 FIELDS1=1.1,1.2,1.3,1.4,1.5,1.6,1.7,1.8,1.9,0
 FIELDS2=2.1,2.2,2.3,2.4,2.5,2.6,2.7,2.8,2.9,0
-egrep -av '^[bc]' $LIST | join -o $FIELDS2 -110 -210 -v2 /dev/null - > $TMP1
+egrep -av '^[bc]' $LIST | \
+       join -t '|' -o $FIELDS2 -110 -210 -v2 /dev/null - > $TMP1
 if [ -s $TMP1 ] ; then
        # Check to make sure uudecode isn't setuid.
        if grep -aw uudecode $TMP1 > /dev/null ; then
@@ -449,23 +452,24 @@
                        :
                else
                        next_part "Setuid additions:"
-                       join -o $FIELDS2 -110 -210 -v2 $CUR $TMP1 | \
-                               tee $TMP2 | column -t
+                       join -t '|' -o $FIELDS2 -110 -210 -v2 $CUR $TMP1 | \
+                               tee $TMP2 | column -s '|' -t
 
                        next_part "Setuid deletions:"
-                       join -o $FIELDS1 -110 -210 -v1 $CUR $TMP1 | \
-                               tee -a $TMP2 | column -t
+                       join -t '|' -o $FIELDS1 -110 -210 -v1 $CUR $TMP1 | \
+                               tee -a $TMP2 | column -s '|' -t
 
                        next_part "Setuid changes:"
-                       sort +9 $TMP2 $CUR $TMP1 | \
-                           sed -e 's/[  ][      ]*/ /g' | uniq -u | column -t
+                       sort -t '|' +9 $TMP2 $CUR $TMP1 | \
+                           sed -e 's/[  ][      ]*/ /g' | uniq -u | \
+                           column -s '|' -t
 
                        cp $CUR $BACK
                        cp $TMP1 $CUR
                fi
        else
                next_part "Setuid additions:"
-               column -t $TMP1
+               column -s '|' -t $TMP1
                cp $TMP1 $CUR
        fi
 fi

> --- security  Wed Jun  3 11:06:07 2009
> +++ /etc/security     Wed Dec 29 15:56:37 2010
> @@ -427,7 +427,9 @@
>       \) -a -prune -o \
>       -type f -a \( -perm -u+s -o -perm -g+s \) -print0 -o \
>       ! -type d -a ! -type f -a ! -type l -a ! -type s -a ! -type p \
> -     -print0 | xargs -0 -r ls -ldgT | sort +9 > $LIST
> +     -print0 | xargs -0 -r ls -ldgT | \
> +     sed 'h;s,[^/]*,,;s,[[:blank:]],_,g;x;s,/.*,,;G;s/\n//' | \
> +     sort +9 > $LIST
>  )
>  
>  # Display any changes in the setuid/setgid file list.
> 

schwa...@usta.de (Ingo Schwarze), 2010.12.30 (Thu) 00:21 (CET):
> Ingo Schwarze wrote on Wed, Dec 29, 2010 at 08:37:16PM +0100:
> > MERIGHI Marcus wrote on Wed, Dec 29, 2010 at 07:43:08PM +0100:
>  
> >> security(8) reports 
> >> ``/home/XXX/Daten/Edv/macs/macs-home/Library/Application''
> >> as ``Setuid additions:'' where the real file name is
> >> ``/home/XXX/Daten/Edv/macs/macs-home/Library/Application Support/\
> >> ProxyOnOff/proxyOnOffTool''
> >> 
> >> I have found the source of the wrong file name report to be in line 437
> >> of /etc/security:
> >> ``egrep -av '^[bc]' $LIST | join -o $FIELDS2 -110 -210 -v2 \
> >> /dev/null - > $TMP1'',
> >> 
> >> with join having space (and tab) characters as field separators and thus
> >> ignoring after first space characters found in field 10.
> 
> > Hmm, i consider that a bug in security(8).
>  
> >> No quick fix that comes to my mind, using -t to join(1) would help only
> >> if the output of ls(1) in line 430 would be changed to not contain space
> >> characters as output separators. 
> 
> > That idea is not bad.
> > Maybe, one could use sed(1) or awk(1) to translate the first ten spaces
> > to null bytes before the join, then translate them back before output,
> > if needed.  Or something similar.
> 
> That didn't work.  It turned out the null byte can only be used as
> a delimiter by utilities explicitly supporting it (like xargs(1)),
> not by random utilities like join(1). 

Quick testing showed you are partly right; ``cut(1) -d "\0"'' does not
work, whereas ``tr(1) "\0" "\n"'' does.

Isn't ignoring nul as delimiter a bug in join(1) and cut(1)?

> And any other replacement
> runs a risk to be contained in file names as well.

That is true. Let's talk about probability. I suggest 174, pipe, ``|''. 

Bye, 

Marcus

Reply via email to