Bug #71390 for Digest: Replace eval STRING with eval BLOCK

Sat Oct 01 16:01:16 2011 mschwern [...] cpan.org - Ticket created

Subject:

Replace eval STRING with eval BLOCK

eval "require $module" is a subtle security hole which can be exploited by a clever attacker and not entirely bullet-proof programming. It should be replaced with `eval { require $module }`. Patch attached. Digest attempts to filter the input to $module, but does so in an incomplete fashion (patch attached to fix). Here is a sample attack against the current version of Digest. use Digest; my $input = q{MD;5;print qq[I own you\n]}; Digest->new($input); If the user is allowed to supply their own algorithm it can be used to execute code. I would recommend this be released quickly and quietly.

Sat Oct 01 16:02:58 2011 mschwern [...] cpan.org - Correspondence added

Forgot the patches.

Subject:

0001-Fix-security-hole-in-new.patch

From 7f8ab1153e243b03db16dd17408affca6b98cb96 Mon Sep 17 00:00:00 2001 From: "Michael G. Schwern" <schwern@pobox.com> Date: Sat, 1 Oct 2011 13:01:41 -0700 Subject: [PATCH] Fix security hole in new(). use Digest; my $input = q{MD;5;print q[I own you]}; Digest->new($input); eval STRING is dangerous and the input filtering was incomplete. --- Digest.pm | 4 ++-- digest-bench | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Digest.pm b/Digest.pm index 384dfc8..8ea4c78 100644 --- a/Digest.pm +++ b/Digest.pm @@ -24,7 +24,7 @@ sub new shift; # class ignored my $algorithm = shift; my $impl = $MMAP{$algorithm} || do { - $algorithm =~ s/\W+//; + $algorithm =~ s/\W+//g; "Digest::$algorithm"; }; $impl = [$impl] unless ref($impl); @@ -35,7 +35,7 @@ sub new ($class, @args) = @$class if ref($class); no strict 'refs'; unless (exists ${"$class\::"}{"VERSION"}) { - eval "require $class"; + eval { require $class }; if ($@) { $err ||= $@; next; diff --git a/digest-bench b/digest-bench index 909e5eb..8e26bf7 100755 --- a/digest-bench +++ b/digest-bench @@ -5,7 +5,7 @@ die unless @ARGV; my($mod, @args) = @ARGV; -eval "require $mod"; die $@ if $@; +eval { require $mod } or die $@; $a = substr(join("", "a" .. "z",) x 800, 0, 8 * 1024); my $count = 4*1024; -- 1.7.6.4

Sat Oct 01 16:02:59 2011 mschwern [...] cpan.org - Status changed from 'new' to 'open'

Sat Oct 01 21:06:22 2011 mschwern [...] cpan.org - Correspondence added

Ok, that patch is wrong. It's a bit distressing that the tests didn't fail. Fixing that.

Sun Oct 02 05:48:52 2011 GAAS [...] cpan.org - Correspondence added

Merged in <https://github.com/gisle/digest/commit/73b8496ea60104282d0bd9e2d282551e70bb1cbd>.

Sun Oct 02 05:48:52 2011 GAAS [...] cpan.org - Status changed from 'open' to 'resolved'