Skip Menu |
 

This queue is for tickets about the Net-DNS CPAN distribution.

Report information
The Basics
Id: 27285
Status: resolved
Priority: 0/
Queue: Net-DNS

People
Owner: Nobody in particular
Requestors: Steffen_Ullrich [...] genua.de
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 0.59
Fixed in: (no value)



Subject: Bugs in dn_expand (XS and PP) on mailformed packages
Download (untitled) / with headers
text/plain 529b
Hi, the XS implementation puts the return code of netdns_dn_expand into an unsigned int instead of an int, so that it never finds out if the function returned an error (e.g. <0). The PP implementation goes into and endless loop exhausting the stack on a mailformed DNS packet, where the string compression causes and endless loop (e.g. the pointer in www.example.<pointer> points to 'www' again etc). Both problems have been fixed in the attached diff which also contains a test for this problem. Regards, Steffen (CPAN SULLR)
Subject: Net-DNS-0.59.patch
Download Net-DNS-0.59.patch
text/x-diff 3.7k
--- Net-DNS-0.59/DNS.xs 2006-09-18 21:22:12.000000000 +0200 +++ Net-DNS-0.59-fixed/DNS.xs 2007-05-25 10:15:21.000000000 +0200 @@ -57,7 +57,7 @@ STRLEN len; u_char * buf; u_char name[MAXDNAME]; - unsigned int pos; + int pos; if (SvROK(sv_buf)) sv_buf = SvRV(sv_buf); --- Net-DNS-0.59/lib/Net/DNS/Packet.pm 2006-09-18 21:22:12.000000000 +0200 +++ Net-DNS-0.59-fixed/lib/Net/DNS/Packet.pm 2007-05-25 10:59:41.000000000 +0200 @@ -752,7 +752,7 @@ } sub dn_expand_PP { - my ($packet, $offset) = @_; # $seen from $_[2] for debugging + my ($packet, $pkt_offset) = @_; # $seen from $_[2] for debugging my $name = ""; my $len; my $packetlen = length $$packet; @@ -760,37 +760,42 @@ # Debugging # warn "USING PURE PERL dn_expand()\n"; - #if ($seen->{$offset}) { - # die "dn_expand: loop: offset=$offset (seen = ", + #if ($seen->{$pkt_offset}) { + # die "dn_expand: loop: offset=$pkt_offset (seen = ", # join(",", keys %$seen), ")\n"; #} - #$seen->{$offset} = 1; + #$seen->{$pkt_offset} = 1; + my $checked = 0; + my $hasPtr = 0; + my $offset = $pkt_offset; while (1) { return (undef, undef) if $packetlen < ($offset + 1); + return (undef, undef) if $checked > $packetlen; # endless Loop $len = unpack("\@$offset C", $$packet); if ($len == 0) { $offset++; + $pkt_offset++ if !$hasPtr; last; } elsif (($len & 0xc0) == 0xc0) { + # pointer into message for compressed strings return (undef, undef) if $packetlen < ($offset + $int16sz); + $pkt_offset+=$int16sz if !$hasPtr; + $checked += $int16sz; my $ptr = unpack("\@$offset n", $$packet); $ptr &= 0x3fff; - my($name2) = dn_expand_PP($packet, $ptr); # pass $seen for debugging - - return (undef, undef) unless defined $name2; - - $name .= $name2; - $offset += $int16sz; - last; + $offset = $ptr; + $hasPtr = 1; + next; # restart with offset from pointer } else { $offset++; + $pkt_offset+=1 if !$hasPtr; return (undef, undef) if $packetlen < ($offset + $len); @@ -800,13 +805,15 @@ $name .= Net::DNS::wire2presentation($elem)."."; $offset += $len; + $pkt_offset+= $len if !$hasPtr; + $checked += $len; } } $name =~ s/\.$//o; - return ($name, $offset); + return ($name, $pkt_offset); } =head2 sign_tsig --- Net-DNS-0.59/MANIFEST 2006-09-18 21:22:12.000000000 +0200 +++ Net-DNS-0.59-fixed/MANIFEST 2007-05-25 11:02:11.000000000 +0200 @@ -94,5 +94,6 @@ t/10-recurse.t t/11-escapedchars.t t/11-inet6.t +t/12-compression.t t/custom.txt TODO --- Net-DNS-0.59/t/12-compression.t 1970-01-01 01:00:00.000000000 +0100 +++ Net-DNS-0.59-fixed/t/12-compression.t 2007-05-25 11:16:43.000000000 +0200 @@ -0,0 +1,33 @@ +# build DNS packet which has an endless loop in compression +# check it against XS and PP implementation of dn_expand +# both should return (undef,undef) as a sign that the packet +# is invalid + +use Test::More tests => 2; +use strict; +use Net::DNS; + +# simple query packet +my $pkt = Net::DNS::Packet->new( 'www.example.com','a' )->data; + +# replace 'com' with pointer to 'example', thus causing +# endless loop for compressed string: +# www.example.example.example.example... +my $pos = pack( 'C', index( $pkt,"\007example" )); +$pkt =~s{\003com}{\xc0$pos\001x}; + +# start at 'www' +my $start_offset = index( $pkt,"\003www" ); + +# fail in case the implementation is buggy and loops forever +$SIG{ ALRM } = sub { BAIL_OUT( "endless loop?" ) }; +alarm(15); + +# XS implementation +skip("No dn_expand_xs available",1) if ! $Net::DNS::HAVE_XS; +my ($name,$offset) = Net::DNS::Packet::dn_expand( \$pkt,$start_offset ); +ok( !defined($name) && !defined($offset), 'XS detected invalid packet' ); + +$Net::DNS::HAVE_XS = 0; +my ($name,$offset) = Net::DNS::Packet::dn_expand( \$pkt,$start_offset ); +ok( !defined($name) && !defined($offset), 'PP detected invalid packet' );
Thanks for the patch... Has been checked into the trunk. --Olaf
published in 0.60


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.