[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apache-modperl
Subject:    RE: perl/hash_attack.t fails with 5.10.1 + CVE-2013-1667 fix
From:       "Steve Hay" <Steve.Hay () verosoftware ! com>
Date:       2013-03-15 17:56:05
Message-ID: 67B2BB40A61BE846B65EF4793B863D6C6642C8 () ukmail02 ! planit ! group
[Download RAW message or body]

Steve Hay wrote on 2013-03-14:
> Niko Tyni wrote on 2013-03-13:
>> On Wed, Mar 13, 2013 at 09:13:15AM -0000, Steve Hay wrote:
>>> Dominic Hargreaves wrote on 2013-03-12:
>> 
>>>> When trying to fix this issue in Debian stable, I found that the
>>>> patch at
>>>> 
>>>> http://svn.apache.org/viewvc?view=revision&revision=1455340
>>>> 
>>>> does not stop the test failing when applied to 2.0.4 (as currently
>>>> found in Debian stable) and built against the current perl package
>>>> in Debian stable (5.10 + the rehashing fix).
>> 
>>> I haven't looked at the Debian package, or tried anything with
>>> mod_perl-2.0.4, but I've just checked out origin/maint-5.10 from
>>> the Perl git repo (in fact, I took the snapshot at
>>> 
>>> 
>>>
http://perl5.git.perl.org/perl.git/snapshot/f14269908e5f8b4cab4b55643d
>>> 7d d9de577e7918.tar.gz) and tried that with Apache 2.2.22 and
mod_perl
>>> from trunk and the tests all pass for me... (This is on Windows 7
x64
>>> with VC++ 2010.)
>> 
>> Thanks for checking.
>> 
>> FWIW, I can reproduce the failure with the Debian perl 5.10.1 package
>> and mod_perl2 2.0.7 with just the above test fix. So it doesn't seem
to
>> be a Debian change that breaks it. Maybe -Dusethreads or something
like
>> that.
>> 
>> I'll keep looking and send an update when I know more.
> 
> 
> The perl I built and tested with was made with ithreads enabled.
> 
> There is an alternative patch to fix this test, submitted to
> mod_perl's rt.cpan.org queue after I'd applied the patch from the
> perl5-security queue on rt.perl.org:
> 
> https://rt.cpan.org/Ticket/Display.html?id=83916
> 
> I haven't tried it myself yet, but is that any better for you?

Zefram has now come up with an even better patch (on the same RT
ticket), after reproducing the Debian 5.10.1 failure himself.

Please take a look (I've also attached it here for your convenience) and
let me know whether this works for you. If so then I hope to apply it to
SVN over the weekend.

["hattack_synthesis.patch" (application/octet-stream)]

--- a/t/response/TestPerl/hash_attack.pm	2013-03-15 13:35:14.000000000 +0000
+++ b/t/response/TestPerl/hash_attack.pm	2013-03-15 13:38:29.000000000 +0000
@@ -5,10 +5,11 @@
 # and fixup handlers in this test). Moreover it must not fail to find
 # that entry on the subsequent requests.
 #
-# the hash attack is detected when HV_MAX_LENGTH_BEFORE_SPLIT keys
-# find themselves in the same hash bucket, in which case starting from
-# 5.8.2 the hash will rehash all its keys using a random hash seed
-# (PL_new_hash_seed, set in mod_perl or via PERL_HASH_SEED environment
+# the hash attack is detected when HV_MAX_LENGTH_BEFORE_REHASH keys find
+# themselves in the same hash bucket on splitting (which happens when the
+# number of keys crosses the threshold of a power of 2), in which case
+# starting from 5.8.2 the hash will rehash all its keys using a random hash
+# seed (PL_new_hash_seed, set in mod_perl or via PERL_HASH_SEED environment
 # variable)
 #
 # Prior to the attack condition hashes use the PL_hash_seed, which is
@@ -29,7 +30,7 @@
 
 use constant MASK_U32  => 2**32;
 use constant HASH_SEED => 0; # 5.8.2: always zero before the rehashing
-use constant THRESHOLD => 14; #define HV_MAX_LENGTH_BEFORE_SPLIT
+use constant THRESHOLD => 14; #define HV_MAX_LENGTH_BEFORE_(SPLIT|REHASH)
 use constant START     => "a";
 
 # create conditions which will trigger a rehash on the current stash
@@ -57,6 +58,8 @@
     return Apache2::Const::OK;
 }
 
+sub buckets { scalar(%{$_[0]}) =~ m#/([0-9]+)\z# ? 0+$1 : 8 }
+
 sub attack {
     my $stash = shift;
 
@@ -74,9 +77,9 @@
     my $bits = $keys ? log($keys)/log(2) : 0;
     $bits = $min_bits if $min_bits > $bits;
 
-    $bits = int($bits) < $bits ? int($bits) + 1 : int($bits);
-    # need to add 2 bits to cover the internal split cases
-    $bits += 2;
+    $bits = ceil($bits);
+    # need to add 3 bits to cover the internal split cases
+    $bits += 3;
     my $mask = 2**$bits-1;
     debug "mask: $mask ($bits)";
 
@@ -90,7 +93,7 @@
         next unless ($h & $mask) == 0;
         $c++;
         $stash->{$s}++;
-        debug sprintf "%2d: %5s, %10s, %s", $c, $s, $h, scalar(%$stash);
+        debug sprintf "%2d: %5s, %08x %s", $c, $s, $h, scalar(%$stash);
         push @keys, $s;
         debug "The hash collision attack has been successful"
             if Internals::HvREHASH(%$stash);
@@ -98,6 +101,24 @@
         $s++;
     }
 
+    # If the rehash hasn't been triggered yet, it's being delayed until the
+    # next bucket split.  Add keys until a split occurs.
+    unless (Internals::HvREHASH(%$stash)) {
+        debug "Will add padding keys until hash split";
+        my $old_buckets = buckets($stash);
+        while (buckets($stash) == $old_buckets) {
+            next if exists $stash->{$s};
+            $h = hash($s);
+            $c++;
+            $stash->{$s}++;
+            debug sprintf "%2d: %5s, %08x %s", $c, $s, $h, scalar(%$stash);
+            push @keys, $s;
+            debug "The hash collision attack has been successful"
+                if Internals::HvREHASH(%$stash);
+            $s++;
+        }
+    }
+
     # this verifies that the attack was mounted successfully. If
     # HvREHASH is on it is. Otherwise the sequence wasn't successful.
     die "Failed to mount the hash collision attack"
@@ -108,6 +129,12 @@
     return @keys;
 }
 
+# least integer >= n
+sub ceil {
+    my $value = shift;
+    return int($value) < $value ? int($value) + 1 : int($value);
+}
+
 # trying to provide the fastest equivalent of C macro's PERL_HASH in
 # Perl - the main complication is that the C macro uses U32 integer
 # (unsigned int), which we can't do it Perl (it can do I32, with 'use


[prev in list] [next in list] [prev in thread] [next in thread]