From 60dc492c5e5d8c4762a9550844114b6f678606fa Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Thu, 17 Nov 2022 20:43:51 -0600 Subject: [PATCH] Add checks to avoid removing backup files in scan_incoming Also update internal documentation and adjust testsuite to properly cover the new edge case. --- gatekeeper.pl | 13 +++++++------ testsuite/gatekeeper.all/00_idle.exp | 7 +++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index 42ea8e6..62f85b4 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -1707,12 +1707,15 @@ sub scan_incoming { 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)$/) { + unless (length($_) <= MAX_FILE_NAME_LEN && m/^($RE_filename_here)$/) { m/^(.*)$/; # untaint the value - push @trash, File::Spec->catfile($directory, $1); $badname_count++; # 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. + unless (m/^[.]$RE_filename_here$/) { + # do not unlink backup files left by cleanup and cleanup_dir + push @trash, File::Spec->catfile($directory, $1); $badname_count++; + } next ENT } my $ent = $1; # if we get here, $RE_filename_here matched above @@ -2599,8 +2602,7 @@ sub execute_commands { Remove all files older than one day from DIRECTORY. As a precaution against data loss, "removed" files are actually simply -renamed to have a leading dot. Note that, for the inbox directory, -scan_incoming will remove those files on the next run. +renamed to have a leading dot. =cut @@ -2635,8 +2637,7 @@ sub cleanup_dir { Remove FILES from the inbox, scratch, and staging directories. As a precaution against data loss, "removed" files are actually simply -renamed to have a leading dot. Note that, for the inbox directory, -scan_incoming will remove those files on the next run. +renamed to have a leading dot. =cut diff --git a/testsuite/gatekeeper.all/00_idle.exp b/testsuite/gatekeeper.all/00_idle.exp index 4b3c5ee..d6223c0 100644 --- a/testsuite/gatekeeper.all/00_idle.exp +++ b/testsuite/gatekeeper.all/00_idle.exp @@ -58,11 +58,13 @@ foreach file { # 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 _x +x + x=x .abcfoobar ..abcfoobar -abcfoobar x;x \\~xax x*x x:x x?x ;xax _x +x } { put_file [file join $tenv incoming ${file}] "bogus input $file\n" age_file [file join $tenv incoming ${file}] "3 minutes ago" } +# ... except for .abcfoobar, which cleanup would produce when removing a +# file named "abcfoobar", which is an acceptable name start_test_services $tenv run_upload_batch_test @@ -71,7 +73,7 @@ stop_test_services analyze_file_tree $tenv "idle processing: bogus files" { incoming } files { - bogus1 bogus2 bogus3 + bogus1 bogus2 bogus3 .abcfoobar bo_gus bo+gus bo-gus bo~gus bo.gus } analyze_file_tree $tenv "idle processing: bogus files" { @@ -95,6 +97,7 @@ analyze_log $tenv "idle processing: bogus files" { !scan,x=x "ignored file: x=x" !scan,.abcfoobar "ignored file: .abcfoobar" + !scan,..abcfoobar "ignored file: ..abcfoobar" !scan,-abcfoobar "ignored file: -abcfoobar" !scan,x;x "ignored file: x;x" !scan,~xax "ignored file: ~xax " -- 2.25.1