containers / podman-security-bench

Apache License 2.0
39 stars 10 forks source link

Fixed regex for suid/sgid files #15

Closed wdyzewski closed 1 year ago

wdyzewski commented 1 year ago

I've created test directory containing files:

$ ls -lh
total 0
-rw-rw-r-- 1 mdyzio mdyzio 0 May  8 20:30 'file with spaces'
-rw-rw-r-- 1 mdyzio mdyzio 0 May  8 20:32 'file with spaces and spaces and number 007'
-rw-rw-r-- 1 mdyzio sudo   0 May  8 20:30  owner-group-has-s
-rw-rw-r-- 1 sys    sys    0 May  8 20:36  owner-name-has-s
-rw-rwSr-- 1 mdyzio mdyzio 0 May  8 20:33  real-sgid-file
-rwSrw-r-- 1 mdyzio mdyzio 0 May  8 20:33  real-suid-file
-rw-rw-r-- 1 mdyzio mdyzio 0 May  8 20:30  regular-file
-rwSrw-r-- 1 root   root   0 May  8 20:34  root-suid-file

And executed your way of testing for suid/sgid files:

$ tar c . | tar -tv | grep -E '^[-rwx].*(s|S).*\s[0-9]'
-rw-rw-r-- mdyzio/sudo       0 2023-05-08 20:30 ./owner-group-has-s
-rwSrw-r-- mdyzio/mdyzio     0 2023-05-08 20:33 ./real-suid-file
-rw-rwSr-- mdyzio/mdyzio     0 2023-05-08 20:33 ./real-sgid-file
-rw-rw-r-- sys/sys           0 2023-05-08 20:36 ./owner-name-has-s
-rw-rw-r-- mdyzio/mdyzio     0 2023-05-08 20:32 ./file with spaces and spaces and number 007
-rwSrw-r-- root/root         0 2023-05-08 20:34 ./root-suid-file

It matched definitely too much files :) You can see on the attached screenshot that s was matched in strange places - not always in the permissions column: Screenshot from 2023-05-08 20-53-31

I reworked regex and put it in awk since it is used as the next piped command:

$ tar c . | tar -tv | awk '/^[-rwx]+[sS]/'
-rwSrw-r-- mdyzio/mdyzio     0 2023-05-08 20:33 ./real-suid-file
-rw-rwSr-- mdyzio/mdyzio     0 2023-05-08 20:33 ./real-sgid-file
-rwSrw-r-- root/root         0 2023-05-08 20:34 ./root-suid-file
$ tar c . | tar -tv | awk '/^[-rwx]+[sS]/ {print $6}'
./real-suid-file
./real-sgid-file
./root-suid-file
rhatdan commented 1 year ago

LGTM Thanks @wdyzewski