Re: Errors when committing to webwml
On Wed, Oct 07, 2009 at 02:28:41AM +0900, victory.deb@gmail.com wrote:
>
> On Tue, 6 Oct 2009 06:57:00 -0700
> Matt Kraai <kraai@ftbfs.org> wrote:
>
> > > Insecure dependency in unlink while running setgid at /cvs/webwml/CVSROOT/log_accum.pl line 63.
>
> 54 sub cleanup_tmpfiles {
> 55 local($wd, @files);
> 57 $wd = `pwd`;
> 58 chdir("$TMP_DIR") || die("Can't chdir('$TMP_DIR')\n");
> 59 opendir(DIR, ".");
> 60 push(@files, grep(/^$FILE_PREFIX\..*\.$id$/, readdir(DIR)));
> 61 closedir(DIR);
> 62 foreach (@files) {
> 63 unlink $_;
> 64 }
> 65 unlink $LAST_FILE . "." . $id;
> 67 chdir($wd);
> 68}
>
> > 63 unlink $_; this line uses returned value ASIS from readdir(DIR).
>
> perl claims about it so i think you'll able to avoid it by doing something like
> s/[^_-\w\d.]//g; (or more narrower) before unlink.
>
> http://perldoc.perl.org/perlsec.html may help..
This page indicates that the only ways to untaint the data are to use
the file names as hash keys and to reference subpatterns from a
regular expression match, so I went with the following patch:
*** log_accum.pl.~1.9.~ 2009-08-29 07:43:42.000000000 -0700
--- log_accum.pl 2009-10-13 02:55:28.000000000 -0700
***************
*** 45,52 ****
$REMOVED_FILE = "$TMP_DIR/#cvs.files.removed";
$LOG_FILE = "$TMP_DIR/#cvs.files.log";
- $FILE_PREFIX = "#cvs.files";
-
#
# Subroutines
#
--- 45,50 ----
***************
*** 57,67 ****
$wd = `pwd`;
chdir("$TMP_DIR") || die("Can't chdir('$TMP_DIR')\n");
opendir(DIR, ".");
! push(@files, grep(/^$FILE_PREFIX\..*\.$id$/, readdir(DIR)));
! closedir(DIR);
! foreach (@files) {
! unlink $_;
}
unlink $LAST_FILE . "." . $id;
chdir($wd);
--- 55,66 ----
$wd = `pwd`;
chdir("$TMP_DIR") || die("Can't chdir('$TMP_DIR')\n");
opendir(DIR, ".");
! for my $file (readdir(DIR)) {
! if (/^(#cvs\.files\.[[:alpha:]]+\.\d+\.$id)\$/) {
! unlink $1;
! }
}
+ closedir(DIR);
unlink $LAST_FILE . "." . $id;
chdir($wd);
I suspect there are further problems, so please let me know if you
continue to see error messages from log_accum.pl.
--
Matt Kraai http://ftbfs.org/
Reply to: