| Subject: | Bugs in dn_expand (XS and PP) on mailformed packages |
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 |
--- 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' );