CVE-2015-3408: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: Module::Signature before 0.74 allows remote attackers to execute arbitrary shell commands via a crafted SIGNATURE file which is not properly handled when generating checksums from a signed manifest.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
Published: 2015-05-19T18:59Z
Repository: https://github.com/audreyt/module-signature
Language: Other
Files changed: Makefile.PL, README, lib/Module/Signature.pm
Commit date: 2015-04-05 16:05:20 +0800
Commit message: * Fix issues reported by John Lightsey

Diff: diff --git a/Makefile.PL b/Makefile.PL
index da2d64e..0309cb5 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -8,10 +8,11 @@ license         'CC0';
 all_from        'lib/Module/Signature.pm';
 readme_from     'lib/Module/Signature.pm';
 repository      'http://github.com/audreyt/module-signature';
 install_script  'script/cpansign';
 build_requires  'Test::More', 0, 'IPC::Run', 0;
+requires        'File::Temp';
 
 # On Win32 (excluding cygwin) we know that IO::Socket::INET,
 # which is needed for keyserver stuff, doesn't work. In fact
 # it potentially hangs forever. So bail out with a N/A on
 # Win32.
diff --git a/README b/README
index 1468877..905b876 100644
--- a/README
+++ b/README
@@ -246,11 +246,11 @@ SEE ALSO
     Module::Install, ExtUtils::MakeMaker, Module::Build
 
     Dist::Zilla::Plugin::Signature
 
 AUTHORS
-    唐鳳 <cpan@audreyt.org>
+    Audrey Tang <cpan@audreyt.org>
 
 CC0 1.0 Universal
     To the extent possible under law, 唐鳳 has waived all copyright and
     related or neighboring rights to Module-Signature.
 
diff --git a/lib/Module/Signature.pm b/lib/Module/Signature.pm
index 159d071..154c592 100644
--- a/lib/Module/Signature.pm
+++ b/lib/Module/Signature.pm
@@ -1,7 +1,7 @@
 package Module::Signature;
-$Module::Signature::VERSION = '0.73_01';
+$Module::Signature::VERSION = '0.74';
 
 use 5.005;
 use strict;
 use vars qw($VERSION $SIGNATURE @ISA @EXPORT_OK);
 use vars qw($Preamble $Cipher $Debug $Verbose $Timeout $AUTHOR);
@@ -56,19 +56,21 @@ $CanKeyRetrieve     = undef;
 sub _cipher_map {
     my($sigtext) = @_;
     my @lines = split /\015?\012/, $sigtext;
     my %map;
     for my $line (@lines) {
+        last if $line eq '-----BEGIN PGP SIGNATURE-----';
+        next if $line =~ /^---/ .. $line eq '';
         my($cipher,$digest,$file) = split " ", $line, 3;
         return unless defined $file;
         $map{$file} = [$cipher, $digest];
     }
     return \%map;
 }
 
 sub verify {
-    my %args = ( skip => 1, @_ );
+    my %args = ( @_ );
     my $rv;
 
     (-r $SIGNATURE) or do {
         warn "==> MISSING Signature file! <==\n";
         return SIGNATURE_MISSING;
@@ -177,10 +179,15 @@ sub _fullcheck {
         } };
 
         ($mani, $file) = ExtUtils::Manifest::fullcheck();
     }
     else {
+        my $_maniskip = &ExtUtils::Manifest::maniskip;
+        local *ExtUtils::Manifest::maniskip = sub { sub {
+            return unless $skip;
+            return $_maniskip->(@_);
+        } };
         ($mani, $file) = ExtUtils::Manifest::fullcheck();
     }
 
     foreach my $makefile ('Makefile', 'Build') {
         warn "==> SKIPPED CHECKING '$_'!" .
@@ -236,19 +243,24 @@ sub _verify_gpg {
     local $SIGNATURE = Win32::GetShortPathName($SIGNATURE)
         if defined &Win32::GetShortPathName and $SIGNATURE =~ /[^-\w.:~\\\/]/;
 
     my $keyserver = _keyserver($version);
 
+    require File::Temp;
+    my $fh = File::Temp->new();
+    print $fh $sigtext;
+    close $fh;
+
     my $gpg = _which_gpg();
     my @quiet = $Verbose ? () : qw(-q --logger-fd=1);
     my @cmd = (
         $gpg, qw(--verify --batch --no-tty), @quiet, ($KeyServer ? (
             "--keyserver=$keyserver",
             ($AutoKeyRetrieve and $version ge '1.0.7')
                 ? '--keyserver-options=auto-key-retrieve'
                 : ()
-        ) : ()), $SIGNATURE
+        ) : ()), $fh->filename
     );
 
     my $output = '';
     if( $Verbose ) {
         warn "Executing @cmd\n";
@@ -256,10 +268,11 @@ sub _verify_gpg {
     }
     else {
         my $cmd = join ' ', @cmd;
         $output = `$cmd`;
     }
+    unlink $fh->filename;
 
     if( $? ) {
         print STDERR $output;
     }
     elsif ($output =~ /((?: +[\dA-F]{4}){10,})/) {
@@ -284,11 +297,11 @@ sub _verify_crypt_openpgp {
 
     require Crypt::OpenPGP;
     my $pgp = Crypt::OpenPGP->new(
         ($KeyServer) ? ( KeyServer => $KeyServer, AutoKeyRetrieve => $AutoKeyRetrieve ) : (),
     );
-    my $rv = $pgp->handle( Filename => $SIGNATURE )
+    my $rv = $pgp->handle( Data => $sigtext )
         or die $pgp->errstr;
 
     return SIGNATURE_BAD if (!$rv->{Validity} and $AutoKeyRetrieve);
 
     if ($rv->{Validity}) {
@@ -307,47 +320,50 @@ sub _read_sigfile {
     my $sigfile = shift;
     my $signature = '';
     my $well_formed;
 
     local *D;
-    open D, $sigfile or die "Could not open $sigfile: $!";
+    open D, "< $sigfile" or die "Could not open $sigfile: $!";
 
     if ($] >= 5.006 and <D> =~ /\r/) {
         close D;
-        open D, $sigfile or die "Could not open $sigfile: $!";
+        open D, '<', $sigfile or die "Could not open $sigfile: $!";
         binmode D, ':crlf';
     } else {
         close D;
-        open D, $sigfile or die "Could not open $sigfile: $!";
+        open D, "< $sigfile" or die "Could not open $sigfile: $!";
     }
 
+    my $begin = "-----BEGIN PGP SIGNED MESSAGE-----\n";
+    my $end = "-----END PGP SIGNATURE-----\n";
     while (<D>) {
-        next if (1 .. /^-----BEGIN PGP SIGNED MESSAGE-----/);
-        last if /^-----BEGIN PGP SIGNATURE/;
-
+        next if (1 .. ($_ eq $begin));
         $signature .= $_;
+        return "$begin$signature" if $_ eq $end;
     }
 
-    return ((split(/\n+/, $signature, 2))[1]);
+    return;
 }
 
 sub _compare {
     my ($str1, $str2, $ok) = @_;
 
     # normalize all linebreaks
+    $str1 =~ s/^-----BEGIN PGP SIGNED MESSAGE-----\n(?:.+\n)*\n//;
     $str1 =~ s/[^\S ]+/\n/g; $str2 =~ s/[^\S ]+/\n/g;
+    $str1 =~ s/-----BEGIN PGP SIGNATURE-----\n(?:.+\n)*$//;
 
     return $ok if $str1 eq $str2;
 
     if (eval { require Text::Diff; 1 }) {
         warn "--- $SIGNATURE ".localtime((stat($SIGNATURE))[9])."\n";
         warn '+++ (current) '.localtime()."\n";
         warn Text::Diff::diff( \$str1, \$str2, { STYLE => 'Unified' } );
     }
     else {
         local (*D, *S);
-        open S, $SIGNATURE or die "Could not open $SIGNATURE: $!";
+        open S, "< $SIGNATURE" or die "Could not open $SIGNATURE: $!";
         open D, "| diff -u $SIGNATURE -" or (warn "Could not call diff: $!", return SIGNATURE_MISMATCH);
         while (<S>) {
             print D $_ if (1 .. /^-----BEGIN PGP SIGNED MESSAGE-----/);
             print D if (/^Hash: / .. /^$/);
             next if (1 .. /^-----BEGIN PGP SIGNATURE/);
@@ -410,13 +426,13 @@ sub _sign_gpg {
     (-e "$sigfile.tmp" and -s "$sigfile.tmp") or do {
         unlink "$sigfile.tmp";
         die "Cannot find $sigfile.tmp, signing aborted.\n";
     };
 
-    open D, "$sigfile.tmp" or die "Cannot open $sigfile.tmp: $!";
+    open D, "< $sigfile.tmp" or die "Cannot open $sigfile.tmp: $!";
 
-    open S, ">$sigfile" or do {
+    open S, "> $sigfile" or do {
         unlink "$sigfile.tmp";
         die "Could not write to $sigfile: $!";
     };
 
     print S $Preamble;
@@ -595,11 +611,11 @@ sub _mkdigest_files {
         unless ( exists $found->{$file} ) {
             warn "No such file: $file\n" if $Verbose;
         }
         else {
             local *F;
-            open F, $file or die "Cannot open $file for reading: $!";
+            open F, "< $file" or die "Cannot open $file for reading: $!";
             if (-B $file) {
                 binmode(F);
                 $obj->addfile(*F);
                 $this_hexdigest = $obj->hexdigest;
             }
@@ -947,11 +963,11 @@ L<Module::Install>, L<ExtUtils::MakeMaker>, L<Module::Build>
 
 L<Dist::Zilla::Plugin::Signature>
 
 =head1 AUTHORS
 
-唐鳳 E<lt>cpan@audreyt.orgE<gt>
+Audrey Tang E<lt>cpan@audreyt.orgE<gt>
 
 =head1 CC0 1.0 Universal
 
 To the extent possible under law, 唐鳳 has waived all copyright and related
 or neighboring rights to Module-Signature.

Vulnerable code:     唐鳳 <cpan@audreyt.org>
$Module::Signature::VERSION = '0.73_01';
    my %args = ( skip => 1, @_ );
        ) : ()), $SIGNATURE
    my $rv = $pgp->handle( Filename => $SIGNATURE )
    open D, $sigfile or die "Could not open $sigfile: $!";
        open D, $sigfile or die "Could not open $sigfile: $!";
        open D, $sigfile or die "Could not open $sigfile: $!";
        next if (1 .. /^-----BEGIN PGP SIGNED MESSAGE-----/);
        last if /^-----BEGIN PGP SIGNATURE/;

    return ((split(/\n+/, $signature, 2))[1]);
        open S, $SIGNATURE or die "Could not open $SIGNATURE: $!";
    open D, "$sigfile.tmp" or die "Cannot open $sigfile.tmp: $!";
    open S, ">$sigfile" or do {
            open F, $file or die "Cannot open $file for reading: $!";
唐鳳 E<lt>cpan@audreyt.orgE<gt>
Fixed code: requires        'File::Temp';
    Audrey Tang <cpan@audreyt.org>
$Module::Signature::VERSION = '0.74';
        last if $line eq '-----BEGIN PGP SIGNATURE-----';
        next if $line =~ /^---/ .. $line eq '';
    my %args = ( @_ );
        my $_maniskip = &ExtUtils::Manifest::maniskip;
        local *ExtUtils::Manifest::maniskip = sub { sub {
            return unless $skip;
            return $_maniskip->(@_);
        } };
    require File::Temp;
    my $fh = File::Temp->new();
    print $fh $sigtext;
    close $fh;

        ) : ()), $fh->filename
    unlink $fh->filename;
    my $rv = $pgp->handle( Data => $sigtext )
    open D, "< $sigfile" or die "Could not open $sigfile: $!";
        open D, '<', $sigfile or die "Could not open $sigfile: $!";
        open D, "< $sigfile" or die "Could not open $sigfile: $!";
    my $begin = "-----BEGIN PGP SIGNED MESSAGE-----\n";
    my $end = "-----END PGP SIGNATURE-----\n";
        next if (1 .. ($_ eq $begin));
        return "$begin$signature" if $_ eq $end;
    return;
    $str1 =~ s/^-----BEGIN PGP SIGNED MESSAGE-----\n(?:.+\n)*\n//;
    $str1 =~ s/-----BEGIN PGP SIGNATURE-----\n(?:.+\n)*$//;
        open S, "< $SIGNATURE" or die "Could not open $SIGNATURE: $!";
    open D, "< $sigfile.tmp" or die "Cannot open $sigfile.tmp: $!";
    open S, "> $sigfile" or do {
            open F, "< $file" or die "Cannot open $file for reading: $!";
Audrey Tang E<lt>cpan@audreyt.orgE<gt>