| CC: | Mark Pizzolato - Info Comm <Mark [...] infocomm.com> |
| Subject: | Bug in Net::DNS::Nameserver.pm Version 1096 |
| Date: | Tue, 17 Sep 2013 09:32:29 -0700 |
| To: | "'bug-Net-DNS [...] rt.cpan.org'" <bug-Net-DNS [...] rt.cpan.org> |
| From: | Mark Pizzolato - Info Comm <Mark [...] infocomm.com> |
I've been using the Net::DNS::Namserver object in an application I wrote a couple of years back.
This app can get queries from arbitrary sources on the internet, and thus may get malicious or otherwise bogus queries.
The app has run quite well for most of the last few years with an unexplained crash happening every few weeks.
I had been using # $Id: Nameserver.pm 835 2009-12-29 20:20:38Z olaf $.
I finally decided to dig into this strange crash and captured the incoming and outgoing traffic to this app. The last query which arrived at the app was a UDP packet which had the UDP payload of "GET / HTTP/1.1" (without the quotes). Clearly this isn't a DNS query and the code which parsed it failed. That failure wasn't exactly what caused the crash. The crash was caused by the fact that the parse returned an undefined object, and the calling code didn't check for that case and proceeded to reference the object as if it had been properly defined.
I changed the original code in the udp_connection which read:
my $query = Net::DNS::Packet->new(\$buf);
my $conn = {
to:
my $query = Net::DNS::Packet->new(\$buf);
if (not defined $query){
print "Ignoring Bad query from $peerhost:$peerport. Closing socket.\n"
if $self->{"Verbose"};
return;
}
my $conn = {
while making this change I realized that the same thing could happen with a TCP query. Crafting such a query would be harder since the length would need to prepended to the bogus query content, but still it could cause a similar crash.
I changed the code in tcp_connection which originally read:
my $query = Net::DNS::Packet->new(\$qbuf);
my $conn = {
sockhost => $sock->sockhost(),
sockport => $sock->sockport(),
peerhost => $sock->peerhost(),
peerport => $sock->peerport()
};
to:
my $query = Net::DNS::Packet->new(\$qbuf);
if (not defined $query) {
print "Bad query from $peer. Closing socket.\n"
if $self->{"Verbose"};
$self->{"select"}->remove($sock);
$sock->close();
delete $self->{"_tcp"}{$sock};
return;
}
my $conn = {
sockhost => $sock->sockhost(),
sockport => $sock->sockport(),
peerhost => $sock->peerhost(),
peerport => $sock->peerport()
};
I then looked at the latest Net::DNS code from CPAN. As it turns out, someone else had done essentially the same thing with the code in udp_connection, but the issue for a tcp query had not been addressed.
The tcp_connection should be fixed also.
- Mark Pizzolato
Message body is not shown because it is too large.