gbarr / perl-Convert-ASN1

encode/decode data using ASN.1 description
http://search.cpan.org/dist/Convert-ASN1/
13 stars 21 forks source link

Unsafe decoding creates infinite loop #14

Closed danaj closed 3 years ago

danaj commented 10 years ago

The following test of decoding unsafe input will make an infinite loop spewing warnings in 0.26:

use Convert::ASN1;
my $asn = Convert::ASN1->new;
$asn->prepare(q<
  [APPLICATION 7] SEQUENCE {
    int INTEGER
  }
>);
my $out;
$out = $asn->decode( pack("H*", "dfccd3fde3") );
$out = $asn->decode( pack("H*", "b0805f92cb") );

I ran random 5-byte strings to find two repeatable examples.

Fix: Add a position check to the two do loops on lines 636 and 690 of _decode.pm:

    do {
      $tag .= substr($_[0],$pos++,1);
      $b = ord substr($tag,-1);
    } while($b & 0x80 && $pos < $end);

This can happen in Convert::PEM when an incorrect password is used. See RT 27574 for an example.

danaj commented 10 years ago

Alternate fix. This seems to fit the existing style slightly better but I haven't seen any examples where it matters.

    do {
      return if $pos >= $end;
      $tag .= substr($_[0],$pos++,1);
      $b = ord substr($tag,-1);
    } while($b & 0x80);

This puts the test in front of the substr call so it happens before the first substr.

Also the "my $n = 1" at line 632 is unused.

I'll try to find time to do a pull request using the first code set.

carnil commented 4 years ago

This issue seem to have CVE-2013-7488 assigned, cf. https://bugzilla.redhat.com/show_bug.cgi?id=1821879

carnil commented 4 years ago

Mentioned corresponding pull request is at https://github.com/gbarr/perl-Convert-ASN1/pull/15

carnil commented 3 years ago

@gbarr do the proposed change look good to be merged?

gbarr commented 3 years ago

@carnil I have not been active with anything Perl for a long time. If anyone wants to take maintainership I would be happy to pass it on