[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
URL:
  <https://savannah.nongnu.org/bugs/?58473>

                 Summary: Trailing dot in hostname results in extraneous label
(and corrupt packet)
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: matevarga
            Submitted on: Sun 31 May 2020 09:31:13 PM UTC
                Category: DNS
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: git head

    _______________________________________________________

Details:

Expected behavior:
------------------

When a hostname ending with a dot are looked up using lwip_gethostbyname(),
then the DNS query's QNAME section will contain the right number of labels.

For example, an A query for foo.barbaz. should result in an UDP packet
fragment like

3foo6barbaz0101

(3 char long label: foo, 6 char long label: barbaz, type: 1, class 1)

Actual behavior
---------------

LWIP considers the trailing dot as a separate label with zero lenght.
Therefore the above example will result in the following packet fragment:

3foo6barbaz00101

(3 char long label: foo, 6 char long label: barbaz, 0 char long label: , type:
1, class 1)

DNS servers like dnsmasq won't be able to parse this, and it'll likely be
interpreted as a * query with class 256. Site-local lookups will break.

I have attached the patch. I intend to submit a unit test as well.



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Sun 31 May 2020 09:31:13 PM UTC  Name:
0001-Does-not-add-an-empty-label-when-encountering-traili.patch  Size: 1KiB  
By: matevarga

<http://savannah.nongnu.org/bugs/download.php?file_id=49191>

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #1, bug #58473 (project lwip):

There should always be a zero-marker at the end of the name, and it is added
at line 827:
http://git.savannah.gnu.org/cgit/lwip.git/tree/src/core/dns.c#n827

Why do you add an extra dot?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #2, bug #58473 (project lwip):

Yes, there needs to be a zero-marker, that's fine. But the current code adds
_two_ zero-markers if the name ends with a dot.

> Why do you add an extra dot?

This is part of the specification.

https://tools.ietf.org/html/rfc1034

"Since a complete domain name ends with the root label, this leads to a
printed form which ends in a dot."


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #3, bug #58473 (project lwip):

And yes, my original comment was incorrect. It should have been like (see the
extra zero-markers):

For example, an A query for foo.barbaz. should result in an UDP packet
fragment like
3foo6barbaz00101
(3 char long label: foo, 6 char long label: barbaz, type: 1, class 1)
Actual behavior
---------------
LWIP considers the trailing dot as a separate label with zero length.
Therefore the above example will result in the following packet fragment:
3foo6barbaz000101

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #4, bug #58473 (project lwip):

Any opinions? The bug is quite clear I think and the patch is extremely
simple.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #5, bug #58473 (project lwip):

I guess it's correct and does no harm, but Erik, do you have any more comments
on this?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #6, bug #58473 (project lwip):

If we just do this change, lookups of the same domain with and without a
trailing dot will not share a hostlist entry but trigger a second lookup as I
understand it. Removing the dot before checking the list of cached entries
seems better to me.

dns_local_lookup already removes an extra dot early:

  hostnamelen = strlen(hostname);
  if (hostname[hostnamelen - 1] == '.') {
    hostnamelen--;
  }

http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/core/dns.c#n486

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #7, bug #58473 (project lwip):

By disabling LWIP_RAND_FOR_FUZZ I got some of them to fail:


$ for i in `ls crashed_inputs/*`; do ./triple_fuzz $i ; echo $?; done
Reading input from file... testing file: "crashed_inputs/001"...
0
Reading input from file... testing file: "crashed_inputs/002"...
0
Reading input from file... testing file: "crashed_inputs/003"...
0
Reading input from file... testing file: "crashed_inputs/004"...
Assertion "mss_local is too small" failed at line 486 in
../../src/core/tcp_out.c
Aborted (core dumped)
134
Reading input from file... testing file: "crashed_inputs/005"...
Assertion "inconsistent oversize vs. space" failed at line 504 in
../../src/core/tcp_out.c
Aborted (core dumped)
134
Reading input from file... testing file: "crashed_inputs/006"...
Assertion "pbuf_free: p->ref > 0" failed at line 753 in ../../src/core/pbuf.c
Aborted (core dumped)
134
Reading input from file... testing file: "crashed_inputs/007"...
Assertion "tcp_slowtimr: persist ticking with empty send buffer" failed at
line 1241 in ../../src/core/tcp.c
Aborted (core dumped)
134
Reading input from file... testing file: "crashed_inputs/008"...
Assertion "Can't split segment into length 0" failed at line 851 in
../../src/core/tcp_out.c
Aborted (core dumped)
134
Reading input from file... testing file: "crashed_inputs/009"...
0


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #58473] Trailing dot in hostname results in extraneous label (and corrupt packet)

Ashley Duncan
Follow-up Comment #8, bug #58473 (project lwip):

Oops - wrong bug, disregard comment #7.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?58473>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel