Finding world-writable files in local directories only

Question

Over at Stack Overflow I asked how to scan a local filesystem ignoring all remote mount points. I only really wanted a push in the right direction and received it. After clacking away on my keyboard I think I might have what I what I need. I ask for an evaluation to determine if it fulfills the requirements:

1. Scan every local directory for world-writable files ignoring "special" directories (/sys, /proc, /dev)
2. Ignore directories
3. Ignore files on any remote filesystems which may be mounted
4. Ignore any files in an "exclude" list

I'm quite certain it could be drastically optimized and any suggestions are welcome as are any suggestions for a better algorithm. However, rewriting the script is secondary to performing the task properly.

I've tested it and I'm fairly confident that I've created what I want. I just need some extra eyes to confirm.

#!/usr/bin/perl

# Directives which establish our execution environment
use warnings;
use strict;
use Fcntl ':mode';
use File::Find;
no warnings 'File::Find';
no warnings 'uninitialized';

# Variables used throughout the script
my $DIR = "/var/log/tivoli/";
my $MTAB = "/etc/mtab";
my $PERMFILE = "world_writable_w_files.txt";
my $TMPFILE = "world_writable_files.tmp";
my $EXCLUDE = "/usr/local/etc/world_writable_excludes.txt";
my $ROOT = "/";
my @devNum;

# Create an array of the file stats for "/"
my @rootStats = stat("${ROOT}");

# Compile a list of mountpoints that need to be scanned
my @mounts;

open MT, "<${MTAB}" or die "Cannot open ${MTAB}, $!";

# We only want the local mountpoints
while (<MT>) {
  if ($_ =~ /ext[34]/) {
    my @line = split;
    push(@mounts, $line[1]);

  }

}

close MT;

# Read in the list of excluded files and create a regex from them
my $regExcld = do {
  open XCLD, "<${EXCLUDE}" or die "Cannot open ${EXCLUDE}, $!\n";
  my @ignore = <XCLD>;
  chomp @ignore;
  local $" = '|';
  qr/@ignore/;

};

# Create a regex containing mountpoint dev numbers to compare file device numbers to.
my $devRegex = do {
  chomp @devNum;
  local $" = '|';
  qr/@devNum/;

};

# Create the output file path if it doesn't already exist.
mkdir("${DIR}" or die "Cannot execute mkdir on ${DIR}, $!") unless (-d "${DIR}");

# Create our filehandle for writing our findings
open WWFILE, ">${DIR}${TMPFILE}" or die "Cannot open ${DIR}${TMPFILE}, $!";

foreach (@mounts) {
  # The anonymous subroutine which is executed by File::Find
  find sub {
    # Is it in a basic (non-special) directory, ...
    return if $File::Find::dir =~ /sys|proc|dev/;

    # ...a regular file, ...
    return unless -f;

    # ...local, ...
    my @dirStats = stat($File::Find::name);
    return unless $dirStats[0] =~ $devRegex;

    # ...and world writable?
    return unless (((stat)[2] & S_IWUSR) && ((stat)[2] & S_IWGRP) && ((stat)[2] & S_IWOTH));

    # Add the file to the list of found world writable files unless it is
    # in the list if exclusions
    print(WWFILE, "$File::Find::name\n") unless ($File::Find::name =~ $regExcld);

  }, $_;

}

close WWFILE;

# If no world-writable files have been found ${TMPFILE} should be zero-size;
# Delete it so Tivoli won't alert
if (-z "${DIR}${TMPFILE}") {
  unlink "${DIR}${TMPFILE}";

} else {
  rename("${DIR}${TMPFILE}","${DIR}${PERMFILE}") or die "Cannot rename file ${DIR}${TMPFILE}, $!";

}

Answer 1

Processing mount points

This can be improved in many ways:

# Compile a list of mountpoints that need to be scanned
my @mounts;
open MT, "<${MTAB}" or die "Cannot open ${MTAB}, $!";

# We only want the local mountpoints
while (<MT>) {
  if ($_ =~ /ext[34]/) {
    my @line = split;
    push(@mounts, $line[1]);
  }
}

close MT;

Using bare names for file handles is not recommended: replace MT with my $MT, and <MT> with <$MT>.

The comment talks about local mount points. That's too vague, when in fact you're looking specifically for ext3 and ext4 filesystems.

You could simplify if ($_ =~ /ext[34]/) as if (/ext[34]/).

You could put the entire while loop inside a map, and it will be still readable.

Putting it together, you can rewrite like this:

open my $MT, "<${MTAB}" or die "Cannot open ${MTAB}, $!";

# Compile a list of mountpoints that need to be scanned
# We only want the ext3 ext4 partitions
my @mounts = map((split)[1], grep(/ext[34]/, <$MT>));

close $MT;

Note that if you didn't find any mount points, the script has nothing to process, so it should exit:

die 'no mount points to process' unless @mounts;

Try to apply these techniques everywhere in the script, especially the bit about filehandlers.

---

# Read in the list of excluded files and create a regex from them
my $regExcld = do {
  open XCLD, "<${EXCLUDE}" or die "Cannot open ${EXCLUDE}, $!\n";
  my @ignore = <XCLD>;
  chomp @ignore;
  local $" = '|';
  qr/@ignore/;
};

As earlier, replace XCLD with my $XCLD or something. You also forgot to close it.

Btw, a common way to shorten my @arr = <$fh>; chomp @arr as chomp(my @arr = <$fh>).

Most importantly, should it be really a fatal error if the file with excluded patterns doesn't exist? It might make sense to make this optional. Perhaps issue a warning instead, something like this:

my $regExcld = do {
  if (open my $fh, "<${EXCLUDE}") {
      chomp(my @ignore = <$fh>);
      close($fh);
      local $" = '|';
      qr/@ignore/;
  } else {
      warn "Cannot open ${EXCLUDE}, $!\n";
      undef;
  }
};

---

This bit seems pointless:

# Create a regex containing mountpoint dev numbers to compare file device numbers to.
my $devRegex = do {
  chomp @devNum;
  local $" = '|';
  qr/@devNum/;
};

Because @devNum is an empty array in your script, you never actually gave it any values.

---

This is quite strange:

# Create the output file path if it doesn't already exist.
mkdir("${DIR}" or die "Cannot execute mkdir on ${DIR}, $!") unless (-d "${DIR}");

You misplaced the die: it will run if "${DIR}" is empty, not if mkdir fails.

Also, there's no need whatsoever to double-quote ${DIR}.

Corrected and simplified:

mkdir($DIR) or die "Cannot execute mkdir on ${DIR}, $!" unless -d $DIR;