[Dnsmasq-discuss] [PATCH] dhcp_release6: the fixed part of an IA_NA is 12 bytes, not 24.

[m.dmitrichenko222 at gmail.com]

From: Mikhail Dmitrichenko <m.dmitrichenko222 at gmail.com>

parse_packet() enters the IA_NA sub-option parser 24 bytes into the
option, but the fixed part of an IA_NA is 12 bytes: IAID, T1, T2.
24 is the payload length of an IAADDR option, as the comment in
create_iaadr_option() notes; create_iana_option() in the same file
already uses 12 when it builds the option.

The parser therefore starts 12 bytes past the first sub-option header,
in the middle of the IPv6 address carried by the IAADDR, and walks the
remainder of the IA_NA as if it were a sub-option list. What it
returns depends on the bits of the address being released.

dnsmasq puts an IA_NA into a Reply to a Release only when it has no
binding for the address, and that IA_NA carries an IAADDR followed by
a Status Code of NoBinding. Feeding such a Reply to parse_packet()
gives, before this change:

  2001:db8::1                -> 1 (UnspecFail)
  fd00::dead:beef            -> 1 (UnspecFail)
  2001:db8:1:2:3:4:5:6       -> 3 (NoBinding)
  fe80::a00:27ff:fe12:3456   -> 1 (UnspecFail)
  2001:db8::d:2:0:1          -> 0 (Success)

Over 200000 random addresses, 199985 returned UnspecFail and only 11
returned the correct NoBinding. The "Error: no binding found" message
is lost in the same way. With the offset corrected every one of these
returns 3 and prints the message.

The bounds checks added in 0375e408 are preserved: option_len is still
validated against the remaining packet length, and the sub-option
parser still receives a length matching the buffer it is handed.

Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222 at gmail.com>
---
 contrib/lease-tools/dhcp_release6.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/lease-tools/dhcp_release6.c b/contrib/lease-tools/dhcp_release6.c
index bbdd412..a795a35 100644
--- a/contrib/lease-tools/dhcp_release6.c
+++ b/contrib/lease-tools/dhcp_release6.c
@@ -339,10 +339,10 @@ int16_t parse_packet(char* buf, size_t len)
 
       if (option_type == IA_NA )
 	{
-	  if (option_len < 24)
+	  if (option_len < 12)
 	    return UNSPEC_FAIL;
 
-	  uint16_t result = parse_iana_suboption(buf + current_pos +24, option_len -24);
+	  uint16_t result = parse_iana_suboption(buf + current_pos + 12, option_len - 12);
 	  if (result)
 	    return result;
 	}
-- 
2.25.1