Re: security(8) setuid checks and space character in file name
Ingo Schwarze
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.
This still mangles the file name, but at least you have a chance
to find it on your disk. Anybody has a better plan?
I already told Marcus on misc to mount that one -o nodev,noexec
and use SUIDSKIP; but that's rather a workaround than a fix.
On misc@, 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.
>
> 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.
>
> Is this known and if yes, would a patch to the man page be accepted?
>
> And no, I do not use space characters voluntarily in file names. It is a
> back up of an osx system.
--- 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.
MERIGHI Marcus
schw...@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.
>
schw...@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
Ted Unangst
> 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.
>
> This still mangles the file name, but at least you have a chance
> to find it on your disk. Anybody has a better plan?
sed it back? assume blank is generally space, replace it with
__omgspace__, then after sorting, replace that with a single space?
It will still screw up for tabs and newlines, but I think spaces are a
lot more common.
Theo de Raadt
It is starting to sound like slip/ppp escape protocol.
A language which is more 8-bit capable should be used.
patrick keshishian
wrote:
this is a half-thought-out idea, but can the script use file inodes so
it doesn't have to futz with the file name?
--patrick
Otto Moerbeek
My guess is that it would be better to sort first and then run xargs.
Something like:
find .... -print0 | sort -z | xargs -0 -L1 ls -ldgT
-Otto
Nicholas Marriott
ending up with a different format from ls:
find .... -print0 | xargs -0 -r stat -f '%-40N %-10Su %-10Sg %Sp' | sort
Nicholas Marriott
MERIGHI Marcus
I'd like that one, by far more dapper; but it does not change a thing
about the abbreviated file name beeing compared and reported by join(1).
Marcus