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

---
 Changes     |  3 +++
 External.pm | 24 ++++++++++++++++++++----
 test.pl     | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/Changes b/Changes
index 283ad4d..f36f0d0 100644
--- a/Changes
+++ b/Changes
@@ -8,6 +8,9 @@ newer versions of Net::Ping::External.
 - Support Debian GNU/kFreeBSD
 - ping location on Darwin
 
+0.16 2017-10-27
+- Fix 10 year old command injection vulnerability
+
 0.15 2014-04-12
 - Better detect Windows ping under Cygwin
 - Add ping output for test 2 if it fails
diff --git a/External.pm b/External.pm
index eb472bd..82bc0b8 100644
--- a/External.pm
+++ b/External.pm
@@ -15,11 +15,26 @@ use Carp;
 use Socket qw(inet_ntoa);
 require Exporter;
 
-$VERSION = "0.15";
+$VERSION = "0.16";
 @ISA = qw(Exporter);
 @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, @_);
@@ -34,7 +49,7 @@ sub ping {
   croak("You must provide a hostname") unless defined $args{host};
   $args{timeout} = 5 unless defined $args{timeout} && $args{timeout} > 0;
 
-  my %dispatch = 
+  my %dispatch =
     (linux    => \&_ping_linux,
      gnukfreebsd => \&_ping_linux, #Debian GNU/kFreeBSD
      mswin32  => \&_ping_win32,
@@ -61,6 +76,7 @@ sub ping {
 
   croak("External ping not supported on your system") unless $subref;
 
+  %args = _clean_args(%args);
   return $subref->(%args);
 }
 
@@ -83,7 +99,7 @@ sub _ping_win32 {
 }
 
 # Mac OS X 10.2 ping does not handle -w timeout now does it return a
-# status code if it fails to ping (unless it cannot resolve the domain 
+# status code if it fails to ping (unless it cannot resolve the domain
 # name)
 # Thanks to Peter N. Lewis for this one.
 sub _ping_darwin {
@@ -201,7 +217,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 --git a/test.pl b/test.pl
index 591f6d4..29c5bc8 100644
--- a/test.pl
+++ b/test.pl
@@ -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";
-- 
2.11.0

