Bug#402316: Patch for hinfo-update

[Stephen Gran]

This one time, at band camp, Neil McGovern said:
> Please find attached a patch which should solve (at least partially)
> this problem.
> 
> * removes -r option from wget.
> * specifies an output file to ensure you don't end up with thousands of
>   files.
> * performs perl syntax check to ensure it's a valid perl file.
> 
> This doesn't fix the security hole, which is a bug all in itself.
+
+   # Check it's a valid perl file
+   system("perl","-c",$destfile,"&>/dev/null");
+   if ($? != 0) {
+   print STDERR "File $destfile is invalid, restoring\n";
+   rename $destfile.".bak",$destfile;



Two notes about your invocation of system:

It's considered better form to pass the arguments to system as an
array, and you don't want the output anyway, so:

my @args = ("perl","-c",$destfile);
my $foo = qx/ @args /; # foo contains the output that is ignorable
   # you can optionally not assign the output, or
   # immediately undef $foo, or just move on

The return value of system is the exit status of the program as returned
by the "wait" call.  To get the actual exit value, shift right by eight.
In this case, there's no difference since you're only looking for 0,
but I always cringe when I see it used bare, as I have made that mistake
before, so:

if ($? >> 8 != 0) {


-- 
 -
|   ,''`.Stephen Gran |
|  : :' :[EMAIL PROTECTED] |
|  `. `'Debian user, admin, and developer |
|`- http://www.debian.org |
 -


signature.asc
Description: Digital signature

Bug#402316: Patch for hinfo-update

[Neil McGovern]

tags 402316 + patch
thanks

Hello,

Please find attached a patch which should solve (at least partially)
this problem.

* removes -r option from wget.
* specifies an output file to ensure you don't end up with thousands of
  files.
* performs perl syntax check to ensure it's a valid perl file.

This doesn't fix the security hole, which is a bug all in itself.

Cheers,
Neil
-- 
* hermanr feels like a hedgehog having sex...
--- hinfo-update	2007-01-05 17:17:05.621210451 +
+++ hinfo-update	2007-01-05 17:56:07.403562701 +
@@ -2,6 +2,7 @@
 # script to fetch current dnsbl.ins.pl and whois.ins.pl
 #
 
+use File::Copy;
 use strict;
 
 my $libdir = '/var/lib/hinfo';
@@ -30,13 +31,14 @@
 
 my $capt = '';
 if (-e $wget && -d $libdir) {
-my $c = "$wget -r -N -nd $verbose -P $libdir $options";
+	foreach (@getlist) {
+		my $c = "$wget -N -nd $verbose -P $libdir $options";
 foreach (@ARGV) {
 $c .= " $_";
 }
-foreach (@getlist) {
-$c .= " $blars$_";
-}
+		my $destfile = $libdir."/".$_;
+		copy($destfile,$destfile.".bak");
+		$c .= " $blars$_ -O ".$destfile;
 $c .= ' 2>&1';
 	print $c."\n";
 open WGET, "-|", $c or die "Could not execute: $c";
@@ -56,7 +58,18 @@
 } else {
 	print STDERR $_ while ($_ = );
 }
+
+		# Check it's a valid perl file
+		system("perl","-c",$destfile,"&>/dev/null");
+		if ($? != 0) {
+			print STDERR "File $destfile is invalid, restoring\n";
+			rename $destfile.".bak",$destfile;
+		} else {
+			unlink $destfile.".bak";
+		}
+	}
 }
+
 print STDERR $capt if ($?);
 
 exit($?);


signature.asc
Description: Digital signature