Addresses CVE-2017-7239: ninka license identification tool: insufficient escaping of external input

4 files changed, 42 insertions, 8 deletions

diff --git a/Changes b/Changes
index 20c30d2..c5478f4 100644
--- a/Changes
+++ b/Changes
@@ -1,3 +1,17 @@
+2017-04-02  Daniel M. German  <dmg@uvic.ca>
+
+        These changes address CVE-2017-7239
+
+	* lib/Ninka.pm: upgraded to version to 1.3.2
+
+	* Makefile.PL: Added IO::CaptureOutput to dependencies
+
+	* CommentExtractor.pm:
+
+	- replaced call to system with a single argument with a list of arguments.
+
+	- replaced the use of open3 with captureoutput to simplify the code
+
 2017-03-26  Daniel M. German  <dmg@uvic.ca>
 
 	* lib/Ninka/CommentExtractor.pm (execute_command): 
diff --git a/Makefile.PL b/Makefile.PL
index b29cf02..0779fbf 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -27,6 +27,8 @@ WriteMakefile(
         'Getopt::Std' => '0',
         'IPC::Open3' => '0',
         'Spreadsheet::WriteExcel' => '0',
+        'IO::CaptureOutput' => '0',
+                  
     },
     TEST_REQUIRES => {
         'File::Temp' => '0',
diff --git a/lib/Ninka.pm b/lib/Ninka.pm
index 292549a..21b2e4b 100644
--- a/lib/Ninka.pm
+++ b/lib/Ninka.pm
@@ -9,7 +9,7 @@ use Ninka::SentenceExtractor;
 use Ninka::SentenceFilter;
 use Ninka::SentenceTokenizer;
 
-our $VERSION = '1.3.1';
+our $VERSION = '1.3.2';
 
 sub process_file {
     my ($input_file, $create_intermediary_files, $verbose) = @_;
diff --git a/lib/Ninka/CommentExtractor.pm b/lib/Ninka/CommentExtractor.pm
index c01d91f..9889aad 100644
--- a/lib/Ninka/CommentExtractor.pm
+++ b/lib/Ninka/CommentExtractor.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use IPC::Open3 'open3';
 use Symbol 'gensym';
+use IO::CaptureOutput qw/capture_exec/;
 
 sub new {
     my ($class, %args) = @_;
@@ -21,11 +22,11 @@ sub new {
 sub execute {
     my ($self) = @_;
 
-    my $command = $self->determine_comments_command();
-    my $comments = execute_command($command);
-    if ($command =~ /^comments/ && length($comments) == 0) {
-        $command = create_head_cmd($self->{input_file}, 700);
-        $comments = execute_command($command);
+    my @command = $self->determine_comments_command();
+    my $comments = execute_command(@command);
+    if ($command[0] =~ /^comments/ && length($comments) == 0) {
+        @command = create_head_cmd($self->{input_file}, 700);
+        $comments = execute_command(@command);
     }
 
     return $comments;
@@ -45,7 +46,7 @@ sub determine_comments_command {
         } elsif ($ext =~ /^(java|c|cpp|h|cxx|c\+\+|cc)$/) {
             my $comments_binary = 'comments';
             if (`which $comments_binary` ne '') {
-                return "$comments_binary -c1 '$input_file' 2> /dev/null";
+                return ($comments_binary, "-c1", $input_file);
             } else {
                 return create_head_cmd($input_file, 400);
             }
@@ -60,10 +61,27 @@ sub determine_comments_command {
 sub create_head_cmd {
     my ($input_file, $count_lines) = @_;
 
-    return "head -$count_lines $input_file";
+    return ("head",  "-$count_lines",  $input_file);
 }
 
 sub execute_command {
+    my (@command) = @_;
+    # make sure we have more than one element in the array
+    #    otherwise system will use the shell to do the execution
+    die "command (@command) seems to be missing parameters" unless (scalar(@command) > 1);
+
+    my ($stdout, $error, $success, $status) = capture_exec( @command );
+
+    my $commandSt = join(' ', @command);
+    die "execution of program [$commandSt] failed: status [$status], error [$error]" if ($status != 0);
+
+    return $stdout;
+}
+
+
+# insecure execute command. Leave here for the time being
+# it is dead code
+sub execute_command_old {
     my ($command) = @_;
 
     if ($command =~ /&/) {