CVE-2017-7239: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: Ninka before 1.3.2 might allow remote attackers to obtain sensitive information, manipulate license compliance scan results, or cause a denial of service (process hang) via a crafted filename.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2017-04-10T15:59Z
Repository: https://github.com/dmgerman/ninka
Language: Perl
Files changed: Changes, Makefile.PL, lib/Ninka.pm, lib/Ninka/CommentExtractor.pm
Commit date: 2017-04-02 13:57:09 -0700
Commit message: Addresses  CVE-2017-7239: ninka license identification tool: insufficient escaping of external input

Diff: diff --git a/Changes b/Changes
index 20c30d2..c5478f4 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,19 @@
+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): 
           Added check to make sure no spurious command is executed
 
diff --git a/Makefile.PL b/Makefile.PL
index b29cf02..0779fbf 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -25,10 +25,12 @@ WriteMakefile(
         'File::Spec::Functions' => '0',
         'File::Temp' => '0',
         'Getopt::Std' => '0',
         'IPC::Open3' => '0',
         'Spreadsheet::WriteExcel' => '0',
+        'IO::CaptureOutput' => '0',
+                  
     },
     TEST_REQUIRES => {
         'File::Temp' => '0',
         'Test::More' => '0.98',
         'Test::Pod' => '1.00',
diff --git a/lib/Ninka.pm b/lib/Ninka.pm
index 292549a..21b2e4b 100644
--- a/lib/Ninka.pm
+++ b/lib/Ninka.pm
@@ -7,11 +7,11 @@ use Ninka::CommentExtractor;
 use Ninka::LicenseMatcher;
 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) = @_;
 
     print STDERR "analysing file [$input_file]\n" if $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
@@ -2,10 +2,11 @@ package Ninka::CommentExtractor;
 
 use strict;
 use warnings;
 use IPC::Open3 'open3';
 use Symbol 'gensym';
+use IO::CaptureOutput qw/capture_exec/;
 
 sub new {
     my ($class, %args) = @_;
 
     my $self = bless({}, $class);
@@ -19,15 +20,15 @@ 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;
 }
 
@@ -43,11 +44,11 @@ sub determine_comments_command {
         } elsif ($ext =~ /^(jl|el)$/) {
             return create_head_cmd($input_file, 400);
         } 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);
             }
         } else {
             return create_head_cmd($input_file, 700);
@@ -58,14 +59,31 @@ 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 =~ /&/) {
         die "illegal file name in command to be executed [$command]";
     }

Vulnerable code: our $VERSION = '1.3.1';
    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);
                return "$comments_binary -c1 '$input_file' 2> /dev/null";
    return "head -$count_lines $input_file";
Fixed code: 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

        'IO::CaptureOutput' => '0',
                  
our $VERSION = '1.3.2';
use IO::CaptureOutput qw/capture_exec/;
    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_binary, "-c1", $input_file);
    return ("head",  "-$count_lines",  $input_file);
    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 {