Hi Todd,

Todd C. Miller wrote on Wed, Oct 07, 2020 at 09:36:33AM -0600:

> The recent changes to the daily security script will result in it
> not traversing file systems where the parent mount point is mounted
> with options nodev,nosuid but the child is mounted with setuid
> enabled.
> 
> For example, if /var/www is a separate file system that allows
> setuid but /var is mounted with nodev and nosuid, security will not
> traverse /var/www.
> 
> 198976b83d9da70f.e /var ffs rw,nodev,nosuid 1 2
> 198976b83d9da70f.f /var/www ffs rw 1 2
> 
> The simplest solution is to pass the list of file systems to traverse
> to File::Find and use the equivalent of find's -xdev option.
>
> Anyone want to double-check my logic? :-)

OK schwarze@;
feel free to use the two nits below, inline.

The patch also works.
On one of my machines, i have:
        /co ffs rw,nodev,nosuid 1 2
        /co/destdir ffs rw,noexec,noperm 1 2
The destdir gets checked while the parent /co does not.
Also, i see no regressions, not even with
        /usr/ports ffs rw,nodev,nosuid 1 2
        /usr/ports/pobj ffs rw,nodev,nosuid,wxallowed 1 2

Yours,
  Ingo


> Index: libexec/security/security
> ===================================================================
> RCS file: /cvs/src/libexec/security/security,v
> retrieving revision 1.40
> diff -u -p -u -r1.40 security
> --- libexec/security/security 17 Sep 2020 06:51:06 -0000      1.40
> +++ libexec/security/security 7 Oct 2020 15:34:14 -0000
> @@ -530,6 +530,7 @@ sub strmode {
>  
>  sub find_special_files {
>       my %skip;
> +     my @fs;

The rest of the file uses the compact notation:

        my (%skip, @fs);

>  
>       %skip = map { $_ => 1 } split ' ', $ENV{SUIDSKIP}
>           if $ENV{SUIDSKIP};
> @@ -541,11 +542,11 @@ sub find_special_files {
>           and return;
>       while (<$fh>) {
>               my ($path, $opt) = /\son\s+(.*?)\s+type\s+\w+(.*)/;
> -             $skip{$path} = 1 if $path &&
> -                 ($opt !~ /local/ ||
> -                  ($opt =~ /nodev/ && $opt =~ /nosuid/));
> +             push(@fs, $path) if $path && $opt =~ /local/ &&

The parentheses are redundant, just

        push @fs, $path if ...

is sufficient.

> +                 !($opt =~ /nodev/ && $opt =~ /nosuid/);
>       }
>       close_or_nag $fh, "mount" or return;
> +     return unless @fs;
>  
>       my $setuid_files = {};
>       my $device_files = {};
> @@ -554,14 +555,19 @@ sub find_special_files {
>       File::Find::find({no_chdir => 1, wanted => sub {
>  
>               if ($skip{$_}) {
> -                     no warnings 'once';
>                       $File::Find::prune = 1;
>                       return;
>               }
>  
>               my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,
>                   $atime, $mtime, $ctime, $blksize, $blocks) = lstat;
> -             unless (defined $dev) {
> +             if (defined $dev) {
> +                     no warnings 'once';
> +                     if ($dev != $File::Find::topdev) {
> +                             $File::Find::prune = 1;
> +                             return;
> +                     }
> +             } else {
>                       nag !$!{ENOENT}, "stat: $_: $!";
>                       return;
>               }
> @@ -592,7 +598,7 @@ sub find_special_files {
>               $file->{size}    = $size;
>               @$file{qw(wday mon day time year)} =
>                   split ' ', localtime $mtime;
> -     }}, '/');
> +     }}, @fs);
>  
>       nag $uudecode_is_setuid, 'Uudecode is setuid.';
>       return $setuid_files, $device_files;

Reply via email to