From: Jacob Bachmeyer Date: Sat, 15 Oct 2022 03:05:01 +0000 (-0500) Subject: Revise directory scanning loop in scan_incoming X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=f1380e60bd82442e2af6ea001691798bfc13451c;p=gatekeeper.git Revise directory scanning loop in scan_incoming Files with unacceptable names are now removed immediately after scanning the incoming files directory; the testsuite is adjusted accordingly. --- diff --git a/gatekeeper.pl b/gatekeeper.pl index 9e9dc50..74eea2e 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -857,26 +857,38 @@ sub scan_incoming { my $directory = shift; my $scratchpad = shift; + local *_; + + my @trash; my %possible; # Get list of all possible files from incoming dir. # - opendir (INCOMING, $directory) + opendir INCOMING, $directory or ftp_die("FATAL opendir($directory) failed: $!"); - while (my $tainted_ent = readdir (INCOMING)) { - # don't look at files with a leading dot or dash, but allow those chars - # subsequently. Omit files containing any other weird characters. - next unless $tainted_ent =~ /^($RE_filename_here)$/; - my $ent = $1; - - # Don't look at files with really long names, either. - next if length ($ent) > MAX_FILE_NAME_LEN; - ftp_syslog('debug', "DEBUG: " - ."uploaded file to check: $ent") if DEBUG; + ENT: while (defined($_ = readdir INCOMING)) { + next ENT if m/^[.]{1,2}$/; # skip . and .. entries + # require acceptable filenames + unless (length($_) <= MAX_FILE_NAME_LEN && /^($RE_filename_here)$/) { + m/^(.*)$/; # untaint the value + push @trash, File::Spec->catfile($directory, $1); + # This is safe for unlink (which is all we will do with @trash) + # because the filename came from a directory entry, so it must be a + # valid filename and cannot indicate directory traversal. + next ENT + } + my $ent = $1; # if we get here, $RE_filename_here matched above + # $_ remains tainted, but $ent is an untainted (and safe) copy + + ftp_syslog('debug', "DEBUG: uploaded file to check: $ent") if DEBUG; $possible{$ent} = 1; } - closedir (INCOMING) + closedir INCOMING or ftp_die("FATAL: closedir($directory) failed: $!"); + # dispose of any garbage files + ftp_syslog('info', "Trashcanned files removed") + if unlink @trash; + # No possible files found, so return before we call lsof return () unless %possible; diff --git a/testsuite/gatekeeper.all/00_idle.exp b/testsuite/gatekeeper.all/00_idle.exp index 75bd000..3b4df0c 100644 --- a/testsuite/gatekeeper.all/00_idle.exp +++ b/testsuite/gatekeeper.all/00_idle.exp @@ -56,7 +56,7 @@ foreach file { age_file [file join $tenv incoming ${file}] "3 minutes ago" } -# files which are to be rejected and ignored +# files which are to be rejected, ignored, and removed foreach file { x=x .abcfoobar -abcfoobar x;x \\~xax x*x x:x x?x ;xax } { @@ -73,7 +73,6 @@ analyze_file_tree $tenv "idle processing: bogus files" { } files { bogus1 bogus2 bogus3 _bogus +bogus __bogus _+bogus _-bogus _~bogus _.bogus - x=x .abcfoobar -abcfoobar x;x \\~xax x*x x:x x?x ;xax } analyze_file_tree $tenv "idle processing: bogus files" { in-stage stage pub archive diff --git a/testsuite/lib/gatekeeper.exp b/testsuite/lib/gatekeeper.exp index d7f6735..9c4daae 100644 --- a/testsuite/lib/gatekeeper.exp +++ b/testsuite/lib/gatekeeper.exp @@ -538,6 +538,11 @@ proc analyze_log { base_dir name assess } { set A(scan,$expect_out(1,string)) 1 exp_continue } + -re {^gatekeeper\[[0-9]+\]: \(Test\)\ + Trashcanned files removed} { + # from scan_incoming, when garbage files are purged + exp_continue + } -re {^gatekeeper\[[0-9]+\]: \(Test\)\ DEBUG: lsof command line: [^\r\n]*} { # from scan_incoming, tracing lsof call