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;