xorl %eax, %eax

CVE-2009-1391: Perl Compress::Raw::Zlib off-by-one Heap Buffer Overflow

with 2 comments

This issue was reported by Leo Bergolth and affects Compress::Raw::Zlib prior to 2.017 release. CCompress::Raw::Zlib is popular Perl module that provides zlib to Perl. Here is the buggy code as seen in Zlib.xs of 2.015 release.

DualType 
inflate (s, buf, output, eof=FALSE)
    Compress::Raw::Zlib::inflateStream  s
    SV *        buf
    SV *        output 
    bool        eof 
    uInt        cur_length = 0;
    uInt        prefix_length = 0;
    uInt        increment = 0;
    STRLEN  stmp    = NO_INIT
    uLong     bufinc = NO_INIT
        ...
    if((s->flags & FLAG_APPEND) != FLAG_APPEND) {
        SvCUR_set(output, 0);
    }
    if (SvLEN(output)) {
        prefix_length = cur_length =  SvCUR(output) ;
        s->stream.next_out = (Bytef*) SvPVbyte_nolen(output) + cur_length;
        increment = SvLEN(output) -  cur_length - 1;
        s->stream.avail_out = increment;
    }
    else {
        s->stream.avail_out = 0;
    }
    s->bytesInflated = 0;
		        
    RETVAL = Z_OK;

    while (RETVAL == Z_OK) {
        if (s->stream.avail_out == 0 ) {
           /* out of space in the output buffer so make it bigger */
           Sv_Grow(output, SvLEN(output) + bufinc) ;
           cur_length += increment ;
           s->stream.next_out = (Bytef*) SvPVbyte_nolen(output) + cur_length ;
           increment = bufinc ;
           s->stream.avail_out = increment;
           bufinc *= 2 ; 
     }

     RETVAL = inflate(&(s->stream), Z_SYNC_FLUSH);
   ...
     }

The first problem appears in the second if clause. There, it checks if there is an output stream buffer using SvLEN(). If this is the case, it retrieves the current length and stores it at cur_length, then it updates the next output stream and sets increment value to be the buffer’s length minus the current length minus one. At last, it updates increment variable to that result. In any other case it assumes that there is not output buffer and sets avail_out to zero. During the while loop we can see that if there is no available output buffer, it reallocates space using sv_Grow() and bufinc’s value. It then updates the current length, the next output stream, increment variable to be equal to bufinc, the buffer itself and finally it doubles bufinc’s size for future allocations. To patch this, they changed the approached by assuming that there is no output buffer at the first place like this:

 }
+   
+    /* Assume no output buffer - the code below will update if there is any available */
+    s->stream.avail_out = 0;
+
+
 if (SvLEN(output)) {

Now, in any case avail_out will be set to zero. Next, they removed the else part as well as the length calculations from the first if clause like this:

 if (SvLEN(output)) {
         prefix_length = cur_length =  SvCUR(output) ;
-        s->stream.next_out = (Bytef*) SvPVbyte_nolen(output) + cur_length;
-        increment = SvLEN(output) -  cur_length - 1;
-        s->stream.avail_out = increment;
-    }
-    else {
-        s->stream.avail_out = 0;

This if statement now includes the following additional code:

+    
+        if (s->flags & FLAG_LIMIT_OUTPUT && SvLEN(output) - cur_length - 1 < bufinc)
+        {
+            Sv_Grow(output, bufinc + cur_length + 1) ;
+        }
+    
+        /* Only setup the stream output pointers if there is spare 
+           capacity in the outout SV
+        */
+        if (SvLEN(output) > cur_length + 1)
+        {
+            s->stream.next_out = (Bytef*) SvPVbyte_nolen(output) + cur_length;
+            increment = SvLEN(output) -  cur_length - 1;
+            s->stream.avail_out = increment;
+        }
 }
+    
+
 s->bytesInflated = 0;

It checks the stream’s flags for FLAG_LIMIT_OUTPUT and the important patch is that it checks that the calculated length is less than bufinc which could result in heap corruption. If both of these return true, it will allocate the required space, notice the +1 which was not present in the previous code. This missing +1 resulted in an off-by-one since zlib will attempt to write a NULL termination character there. Continuing with the patch, it makes the appropriate calculations of the output is larger than the current length plus one. However, the same allocation takes place also inside the while loop shown earlier. For this reason, this was patched like this:

 /* out of space in the output buffer so make it bigger *\
-            Sv_Grow(output, SvLEN(output) + bufinc) ;
+            Sv_Grow(output, SvLEN(output) + bufinc +1);
 cur_length += increment ;

Leo Bergolth also noticed that this issue is already exploited in the wild through an email virus which uses this corruption for spamassassin and amavisd-new. He also provides three simple PoC Perl scripts that attempt to unpack, compress and archive to zip ecard.zip and ecard.exe virus which you can find here respectively. The archive script is this:

#!/usr/bin/perl

use Archive::Zip qw(:CONSTANTS :ERROR_CODES);

use strict;

my $zip = Archive::Zip->new;
unless ( $zip->read( 'ecard.zip' ) == AZ_OK ) {
  die;
}

my $mem = $zip->memberNamed( 'ecard.exe' );
unless ($mem) {
  die;
}

my $meth = $mem->compressionMethod;
if ($meth == COMPRESSION_DEFLATED) {
  print "COMPRESSION_DEFLATED\n";
} elsif ($meth == COMPRESSION_STORED) {
  print "COMPRESSION_STORED\n";
}
$mem->desiredCompressionMethod(COMPRESSION_STORED);
unless ($mem->rewindData == AZ_OK ) {
  die;
}
my $status = AZ_OK;
my $bufferRef;
while ($status == AZ_OK) {
  print "reading chunk...\n";
  ($bufferRef, $status ) = $mem->readChunk();
  print "read ".length($$bufferRef)." bytes.\n";
}
unless ($status == AZ_STREAM_END) {
  die "status: $status";
}
$mem->endRead();

Next, the unpacking from ZIP is this:

#!/usr/bin/perl

use Archive::Zip qw(:CONSTANTS :ERROR_CODES);

use strict;

my $zip = Archive::Zip->new;
unless ( $zip->read( 'ecard.zip' ) == AZ_OK ) {
  die;
}

my $mem = $zip->memberNamed( 'ecard.exe' );
unless ($mem) {
  die;
}

my $meth = $mem->compressionMethod;
if ($meth == COMPRESSION_DEFLATED) {
  print "COMPRESSION_DEFLATED\n";
} elsif ($meth == COMPRESSION_STORED) {
  print "COMPRESSION_STORED\n";
}
# $mem->desiredCompressionMethod(COMPRESSION_STORED);
unless ($mem->rewindData == AZ_OK ) {
  die;
}
my $status = AZ_OK;
my $bufferRef;

local *F;
open(F, "> ecard.deflated") or die;

while ($status == AZ_OK) {
  print "reading chunk...\n";
  ($bufferRef, $status ) = $mem->readChunk();
  print F $$bufferRef;
  print "read ".length($$bufferRef)." bytes.\n";
}
unless ($status == AZ_STREAM_END) {
  die "status: $status";
}
close(F);
$mem->endRead();

And at last, the compressing script is this:

#!/usr/bin/perl

use strict ;
use warnings ;

# perl -I Compress-Raw-Zlib-2.020/blib/lib -I Compress-Raw-Zlib-2.020/blib/arch compress-raw-zlib.pl
use Compress::Raw::Zlib qw(Z_OK Z_DATA_ERROR Z_STREAM_END Z_FINISH MAX_WBITS);

# only fails with this chunksize
my $ChunkSize = 32768;

my $x = new Compress::Raw::Zlib::Inflate(-WindowBits => -MAX_WBITS(), -Bufsize => $ChunkSize)
 or die "Cannot create a inflation stream\n" ;

local *IN;
local *OUT;

open(IN, "ecard.deflated") or die;
open(OUT, "> ecard.inflated") or die;

my $input = '' ;
my ($output, $status) ;

while (read(IN, $input, $ChunkSize)) {
 print "inflating...\n";
 $status = $x->inflate(\$input, $output) ;
 print "done: status: $status\n";
 print OUT $output if ($status == Z_OK or $status == Z_STREAM_END);
 last if $status != Z_OK ;
}

die "inflation failed: $status\n"
 unless $status == Z_STREAM_END ;

close(IN);
close(OUT);

Written by xorl

June 13, 2009 at 14:32

Posted in bugs

2 Responses

Subscribe to comments with RSS.

  1. wordpress messed up your code.
    specifically, -> became -&gt
    antiXSS FTW!

    thanasisk

    June 17, 2009 at 07:56

  2. Thanks, in the HTML editing thingie of wordpress they are correct. That’s retarded… It is even in a [sourcecode] wordpress tag…

    xorl

    June 17, 2009 at 16:59


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s