| Subject: | Ensure that tempdirs created during testing are removed in Perl 5.28 |
File::Temp::tempdir() internally calls File::Path::rmtree() to implement the 'CLEANUP => 1' option. A security vulnernability was discovered in rmtree() in early 2017. This vulnerability has been addressed in File::Path versions 2.14 and later; a thus patched version of File::Path will appear in Perl-5.28 to be released in spring 2018.
The security correction changes the functionality of rmtree() when not called with a true value as its third positional argument:
#####
rmtree($my_tempdir, 0, 0);
#####
As of File::Path 2.14, this syntax will no longer remove directories for which the user lacks both 'read' and 'execute' permissions. For example, any directory whose numerical permissions are octal 0000 or 0200 will no longer be deleted.
Within test_permissions(), there are two blocks each of which contains a mkdir() call with 0000 permisssions. In the absence of application of this patch, test_permissions() will continue to return a true value, but the 0000 directories will not be deleted and the person running the test will be shown warnings that some directories have not been deleted.
For the two blocks within test_permissions(), the patch avoids File::Temp::tempdir's internal call to File::Path::rmtree and substitutes manual creation of a temporary directory for each block via File::Spec->tmpdir(). (Unlike File::Temp, File::Spec has no dependency on File::Path::rmtree().) After the existing checks on the tie are run, the patch restores read/write/execute permissions to the temporary directory. This in turn allows the use of File::Path::rmtree($dir, 0, 1) -- note the true value for the true parameter -- to ensure removal of the temporary directories and the elimination of potentially confusing warnings.
The above should guarantee that this distribution's tests will run warnings-free in perl-5.27.3 and later, i.e., with File::Path version 2.14 or later.
Within test_permissions() there are other test blocks not affected by the above considerations. In those blocks the patch changes:
#####
my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
#####
to:
#####
my $filename = tempdir( CLEANUP => 1 );
#####
In File::Temp's documentation, the 'DIR => $somedirectory' syntax is documented as pertinent to the syntax of tempfile(); it is not pertinent to the syntax of tempdir(). The previous code had the probably unintended effect of creating an extra level of temporary directories.
The patch also provide descriptions for each test.
| Subject: | 0001-Ensure-that-tempdirs-created-during-testing-are-remo.patch |
From 96480030e06260fa0b6364a179f7e9c06e1c60ad Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Mon, 14 Aug 2017 16:19:57 -0400
Subject: [PATCH] Ensure that tempdirs created during testing are removed in
Perl 5.28.
File::Temp::tempdir() internally calls File::Path::rmtree() to implement the
'CLEANUP => 1' option. A security vulnernability was discovered in rmtree()
in early 2017. This vulnerability has been addressed in File::Path versions
2.14 and later; a thus patched version of File::Path will appear in Perl-5.28
to be released in spring 2018.
The security correction changes the functionality of rmtree() when not called
with a true value as its third positional argument:
rmtree($my_tempdir, 0, 0);
As of File::Path 2.14, this syntax will no longer remove directories for which
the user lacks both 'read' and 'execute' permissions. For example, any
directory whose numerical permissions are octal 0000 or 0200 will no longer be
deleted.
Within test_permissions(), there are two blocks each of which contains a
mkdir() call with 0000 permisssions. In the absence of application of this
patch, test_permissions() will continue to return a true value, but the 0000
directories will not be deleted and the person running the test will be shown
warnings that some directories have not been deleted.
For the two blocks within test_permissions(), the patch avoids
File::Temp::tempdir's internal call to File::Path::rmtree and substitutes
manual creation of a temporary directory for each block via
File::Spec->tmpdir(). (Unlike File::Temp, File::Spec has no dependency on
File::Path::rmtree().) After the existing checks on the tie are run, the
patch restores read/write/execute permissions to the temporary directory.
This in turn allows the use of File::Path::rmtree($dir, 0, 1) -- note the true
value for the true parameter -- to ensure removal of the temporary directories
and the elimination of potentially confusing warnings.
The above should guarantee that this distribution's tests will run
warnings-free in perl-5.27.3 and later, i.e., with File::Path version 2.14 or
later.
Within test_permissions() there are other test blocks not affected by the
above considerations. In those blocks the patch changes:
my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
to:
my $filename = tempdir( CLEANUP => 1 );
In File::Temp's documentation, the 'DIR => $somedirectory' syntax is
documented as pertinent to the syntax of tempfile(); it is not pertinent to
the syntax of tempdir(). The previous code had the probably unintended effect
of creating an extra level of temporary directories.
The patch also provide descriptions for each test.
---
t/01_Tie-DB_File-SplitHash.t | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/t/01_Tie-DB_File-SplitHash.t b/t/01_Tie-DB_File-SplitHash.t
index b5da057..574759d 100644
--- a/t/01_Tie-DB_File-SplitHash.t
+++ b/t/01_Tie-DB_File-SplitHash.t
@@ -10,6 +10,8 @@ use Tie::DB_File::SplitHash;
#########################
# change 'tests => 9' to 'tests => last_test_to_print';
+use File::Spec;
+use File::Path qw( rmtree );
eval {
require File::Temp;
File::Temp->import('tempdir');
@@ -39,19 +41,19 @@ my $TESTDIR = tempdir(CLEANUP => 1);
#########
# Test 1
-ok (test_tie());
+ok (test_tie(), "test_tie() returned true value");
#########
# Test 2
-ok (test_tied_hash());
+ok (test_tied_hash(), "test_tied_hash() returned true value");
#########
# Test 3
-ok (test_obj_hash());
+ok (test_obj_hash(), "test_obj_hash() returned true value");
#########
# Test 4
-ok (test_permissions());
+ok (test_permissions(), "test_permissions() returned true value");
exit;
@@ -68,7 +70,7 @@ sub test_directory {
sub test_permissions {
{
- my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
+ my $filename = tempdir( CLEANUP => 1 );
my $multi_n = 4;
my $flags = &O_RDWR() | &O_CREAT();
my $mode = 0666;
@@ -87,15 +89,21 @@ sub test_permissions {
}
}
+ my $seed = 0;
{
- my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
+ my $tdir = File::Spec->catdir(
+ File::Spec->tmpdir(),
+ $$ . sprintf("%02s" => ++$seed),
+ );
+ mkdir($tdir, 0700) || return "could not create tmpdir $tdir";
my $multi_n = 4;
my $flags = &O_RDWR() | &O_CREAT();
my $mode = 0666;
my %hash;
+ my $extra_dir;
my $result = eval {
- my $extra_dir = File::Spec->catdir($filename,'extra');
+ $extra_dir = File::Spec->catdir($tdir,'extra');
mkdir($extra_dir, 0000) || return "could not create scratch directory $extra_dir: $!";
my $final_dir = File::Spec->catdir($extra_dir,'subdir');
my $db = tie %hash, 'Tie::DB_File::SplitHash', $final_dir, $flags, $mode, $DB_HASH, $multi_n;
@@ -108,17 +116,25 @@ sub test_permissions {
diag("did not detect failure to tie database due to directory permissions");
return 0;
}
+ chmod 0700, $extra_dir;
+ File::Path::rmtree($tdir, 0, 1);
+ (! -d $tdir) or warn "Directory $tdir not removed";
}
{
- my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
+ my $tdir = File::Spec->catdir(
+ File::Spec->tmpdir(),
+ $$ . sprintf("%02s" => ++$seed),
+ );
+ mkdir($tdir, 0700) || return "could not create tmpdir $tdir";
my $multi_n = 4;
my $flags = &O_RDWR() | &O_CREAT();
my $mode = 0666;
my %hash;
+ my $extra_dir;
my $result = eval {
- my $extra_dir = File::Spec->catdir($filename,'extra');
+ $extra_dir = File::Spec->catdir($tdir,'extra');
unless (mkdir($extra_dir, 0000)) {
diag("failed to make test directory $extra_dir");
die;
@@ -133,10 +149,13 @@ sub test_permissions {
diag("did not detect failure to tie database in forbidden directory");
return 0;
}
+ chmod 0700, $extra_dir;
+ File::Path::rmtree($tdir, 0, 1);
+ (! -d $tdir) or warn "Directory $tdir not removed";
}
{
- my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
+ my $filename = tempdir( CLEANUP => 1 );
my $multi_n = 4;
my $flags = &O_RDWR() | &O_CREAT();
my $mode = 0666;
@@ -153,7 +172,7 @@ sub test_permissions {
}
{
- my $filename = tempdir( DIR => test_directory(), CLEANUP => 1 );
+ my $filename = tempdir( CLEANUP => 1 );
my $multi_n = 4;
my $flags = &O_RDWR() | &O_CREAT();
my $mode = 0666;
--
2.7.4