Blob Blame History Raw
Originally

From b3dd4de6417f5f9d06710fc185ae6b4ee3661c45 Mon Sep 17 00:00:00 2001
From: Matthias Weckbecker <matthias@weckbecker.name>
Date: Fri, 27 Oct 2017 15:01:10 +0200
Subject: [PATCH 1/1] Fix 10 year old command injection vulnerability

slightly edited to apply, and to drop the version bump.

diff -urp Net-Ping-External/External.pm Net-Ping-External-0.15/External.pm
--- Net-Ping-External/External.pm	2014-04-12 21:52:13.000000000 +0200
+++ Net-Ping-External-0.15/External.pm	2017-11-08 16:01:24.597772931 +0100
@@ -19,6 +19,21 @@ $VERSION = "0.15";
 @EXPORT = qw();
 @EXPORT_OK = qw(ping);
 
+sub _clean_args {
+  my %args = @_;
+  for my $arg (qw(size count timeout)) {
+      if ($args{$arg} !~ /([0-9]+)/) {
+        croak("$arg must be numeric");
+      }
+      $args{$arg} = $1;
+  }
+  if ($args{host} !~ /([A-Z0-9\.\-]+)/i) {
+    croak("invalid host");
+  }
+  $args{host} = $1;
+  return %args;
+}
+
 sub ping {
   # Set up defaults & override defaults with parameters sent.
   my %args = (count => 1, size => 56, @_);
@@ -59,6 +74,7 @@ sub ping {
 
   croak("External ping not supported on your system") unless $subref;
 
+  %args = _clean_args(%args);
   return $subref->(%args);
 }
 
@@ -192,7 +208,7 @@ sub _ping_netbsd {
 # -s size option supported -- superuser only... fixme
 sub _ping_bsd {
   my %args = @_;
-  my $command = "ping -c $args{count} -q $args{hostname}";
+  my $command = "ping -c $args{count} -q $args{host}";
   return _ping_system($command, 0);
 }
 
diff -urp Net-Ping-External/test.pl Net-Ping-External-0.15/test.pl
--- Net-Ping-External/test.pl	2014-04-11 02:13:02.000000000 +0200
+++ Net-Ping-External-0.15/test.pl	2017-11-08 16:01:52.661826123 +0100
@@ -6,7 +6,7 @@
 # Change 1..1 below to 1..last_test_to_print .
 # (It may become useful if the test is moved to ./t subdirectory.)
 
-BEGIN { $| = 1; $num_tests = 6; print "1..$num_tests\n"; }
+BEGIN { $| = 1; $num_tests = 8; print "1..$num_tests\n"; }
 END {print "not ok 1\n" unless $loaded;}
 use Net::Ping::External qw(ping);
 $loaded = 1;
@@ -24,7 +24,12 @@ $Net::Ping::External::DEBUG_OUTPUT = 1;
 	       3 => "ping(host => '127.0.0.1', timeout => 5)",
 	       4 => "ping(host => 'some.non.existent.host.')",
 	       5 => "ping(host => '127.0.0.1', count => 10)",
-	       6 => "ping(host => '127.0.0.1', size => 32)"
+	       6 => "ping(host => '127.0.0.1', size => 32)",
+	       7 => "ping(host => '127.0.0.1\$(evil stuff)')",
+	       8 => "ping(host => '127.0.0.1', "
+                      . "count => '1\$(evil stuff)', "
+                      . "size => '1\$(evil stuff)', "
+                      . "timeout => '1\$(evil stuff)')"
 	      );
 
 @passed = ();
@@ -102,6 +107,50 @@ else {
   push @failed, 6;
 }
 
+if ($^O !~ /win/i || $^O eq 'cygwin') {
+  use File::Temp;
+
+  {
+    my $temp = File::Temp->new()->filename();
+    my $evil = sprintf '127.0.0.1$(touch %s)', $temp;
+    eval { ping(host => $evil) };
+    unless (-e $temp) {
+      print "ok 7\n";
+      push @passed, 7;
+    }
+    else {
+      unlink $temp;
+      print "not ok 7\n";
+      push @failed, 7;
+    }
+  }
+  {
+    my $temp = File::Temp->new()->filename();
+    my $evil = sprintf '1$(touch %s)', $temp;
+    my $fail = 0;
+    for (qw(size count timeout)) {
+      eval { ping(host => '127.0.0.1', $_ => $evil) };
+      $fail = 1 if -e $temp;
+    }
+    unless ($fail) {
+      print "ok 8\n";
+      push @passed, 8;
+    }
+    else {
+      unlink $temp;
+      print "not ok 8\n";
+      push @failed, 8;
+    }
+  }
+}
+else {
+  # TODO: win32 tests
+  for (qw(7 8)) {
+    print "ok $_\n";
+    push @passed, $_;
+  }
+}
+
 print "\nRunning a more verbose test suite.";
 print "\n-------------------------------------------------\n";
 print "Net::Ping::External version: ", $Net::Ping::External::VERSION, "\n";