From 81f185261c8863c5b84344ee31192870be939faf Mon Sep 17 00:00:00 2001 From: D German Date: Sun, 2 Apr 2017 13:57:09 -0700 Subject: Addresses CVE-2017-7239: ninka license identification tool: insufficient escaping of external input --- Changes | 14 ++++++++++++++ Makefile.PL | 2 ++ lib/Ninka.pm | 2 +- lib/Ninka/CommentExtractor.pm | 32 +++++++++++++++++++++++++------- 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 + + 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 * 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 =~ /&/) { -- cgit v1.2.1