TXT record decoding bug in inet_dns

[Geoff Cant-2]

Hi there, while sneakily using the private inet_res API to do
interesting DNS queries, I discovered a long-standing bug in inet_dns.

inet_dns incorrectly decodes the RData for TXT records as RData instead
of [Length | Text] = RData, Text. (The RData for TXT records is encoded
as <<Len:8, Text:Len/binary>>).

Attached is a patch against R12B-4 to correct this bug. It should apply
with patch -p1 from the OTP sources root.

diff --git a/lib/kernel/src/inet_dns.erl b/lib/kernel/src/inet_dns.erl
index b99b774..71fd272 100644
--- a/lib/kernel/src/inet_dns.erl
+++ b/lib/kernel/src/inet_dns.erl
@@ -400,7 +400,9 @@ decode_data(?S_MX, _, [P1,P0 | Dom], Buffer) ->
     { ?i16(P1,P0), decode_domain(Dom, Buffer) };
 decode_data(?S_SRV, _, [P1,P0, W1,W0, Po1,Po0 | Dom], Buffer) ->
     { ?i16(P1,P0), ?i16(W1,W0), ?i16(Po1,Po0), decode_domain(Dom, Buffer) };
-decode_data(?S_TXT, _, Data, _Buffer) -> Data;
+decode_data(?S_TXT, _, Data, _Buffer) ->
+    {Text,Rest} = get_data(hd(Data), tl(Data)),
+    Text;
 %% sofar unknown or non standard
 decode_data(_, _, Data, _Buffer) ->
     Data.


The second patch fixes the encoding of TXT records in the same way.

diff --git a/lib/kernel/src/inet_dns.erl b/lib/kernel/src/inet_dns.erl
index 71fd272..a1f10e6 100644
--- a/lib/kernel/src/inet_dns.erl
+++ b/lib/kernel/src/inet_dns.erl
@@ -500,7 +500,8 @@ encode_data(?S_MX, in, {Pref, Exch}, Ptrs, L) ->
 encode_data(?S_SRV, in, {Prio, Weight, Port, Target}, Ptrs, L) ->
     {EDom, NPtrs} = dn_compress(Target, Ptrs, [], L),
     {?int16(Prio) ++ ?int16(Weight) ++ ?int16(Port) ++ EDom, NPtrs};
-encode_data(?S_TXT, in, Data, Ptrs, _)     -> {Data, Ptrs};
+encode_data(?S_TXT, in, Data, Ptrs, _)     ->
+    {[length(Data) | Data], Ptrs};
 %% sofar unknown or non standard
 encode_data(_, _, Data, Ptrs, _)        -> {Data, Ptrs}.
 



On a related note, I've found the functionality in inet_res to be
particularly useful and would like to know if the OTP team would
consider exposing and this functionality. This could be done as a new
'dns' modules for queries, resolution, encoding and decoding or perhaps
just exporting and documenting some of these private inet_res, inet_dns functions.

Ideally I would like to be able to do:

dns:lookup(Name, Type) -> {error, Error} | {ok, RR}.
dns:lookup(Server, Name, Type)
dns:lookup(Server, Name, Type, Timeout)

For at least the SOA, NS, TXT, A, CName, PTR, SRV and MX resource record
types.

I'd be happy to write patches or a new dns module if the OTP team would
look favourably on including it in OTP.

Cheers,
--
Geoff Cant

_______________________________________________
erlang-patches mailing list
erlang-patches@...
http://www.erlang.org/mailman/listinfo/erlang-patches

[Geoff Cant-3]

"Matthew Dempsky" <matthew.dempsky@...> writes:

> 2008/10/6 Geoff Cant <nem@...>:
>> inet_dns incorrectly decodes the RData for TXT records as RData instead
>> of [Length | Text] = RData, Text. (The RData for TXT records is encoded
>> as <<Len:8, Text:Len/binary>>).
>
> RDATA for TXT records allows more than one character string.

Good point - in which case txt records should decode to a list of
strings. Updated patch attached (encoding and decoding) against R12B-4
(supercedes previous patches).


diff --git a/lib/kernel/src/inet_dns.erl b/lib/kernel/src/inet_dns.erl
index b99b774..6bd7867 100644
--- a/lib/kernel/src/inet_dns.erl
+++ b/lib/kernel/src/inet_dns.erl
@@ -400,11 +400,20 @@ decode_data(?S_MX, _, [P1,P0 | Dom], Buffer) ->
     { ?i16(P1,P0), decode_domain(Dom, Buffer) };
 decode_data(?S_SRV, _, [P1,P0, W1,W0, Po1,Po0 | Dom], Buffer) ->
     { ?i16(P1,P0), ?i16(W1,W0), ?i16(Po1,Po0), decode_domain(Dom, Buffer) };
-decode_data(?S_TXT, _, Data, _Buffer) -> Data;
+decode_data(?S_TXT, _, Data, _Buffer) ->
+    decode_txt(Data);
 %% sofar unknown or non standard
 decode_data(_, _, Data, _Buffer) ->
     Data.
 
+decode_txt_data(List) ->
+    decode_txt_data(List, []).
+
+decode_txt_data([], Acc) -> Acc;
+decode_txt_data([Len | Data], Acc) ->
+    {Str,Rest} = lists:split(Len, Data),
+    decode_txt(Rest, Acc ++ [Str]).
+
 decode_domain(Data, Buffer) ->
     case dn_expand(Data, Buffer) of
  error -> error;
@@ -498,7 +507,8 @@ encode_data(?S_MX, in, {Pref, Exch}, Ptrs, L) ->
 encode_data(?S_SRV, in, {Prio, Weight, Port, Target}, Ptrs, L) ->
     {EDom, NPtrs} = dn_compress(Target, Ptrs, [], L),
     {?int16(Prio) ++ ?int16(Weight) ++ ?int16(Port) ++ EDom, NPtrs};
-encode_data(?S_TXT, in, Data, Ptrs, _)     -> {Data, Ptrs};
+encode_data(?S_TXT, in, Data, Ptrs, _)     ->
+    {[[length(Str) | Str] || Str <- Data], Ptrs};
 %% sofar unknown or non standard
 encode_data(_, _, Data, Ptrs, _)        -> {Data, Ptrs}.
 


Cheers,
--
Geoff Cant

_______________________________________________
erlang-patches mailing list
erlang-patches@...
http://www.erlang.org/mailman/listinfo/erlang-patches