buglwIP - A Lightweight TCP/IP stack - Bugs: bug #58473, Trailing dot in hostname results...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  Mate Varga <matevarga>
Submitted:  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 Planned Release:  None
lwIP version:  git head

Jump to the original submission

Fri 19 Jun 2020 11:13:40 AM UTC, comment #8: 

Oops - wrong bug, disregard comment #7.

Erik Ekman <yarrick>
Group Member
Fri 19 Jun 2020 11:12:48 AM UTC, comment #7: 

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

Erik Ekman <yarrick>
Group Member
Fri 19 Jun 2020 10:14:47 AM UTC, comment #6: 

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

Erik Ekman <yarrick>
Group Member
Wed 17 Jun 2020 07:56:48 PM UTC, comment #5: 

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

Simon Goldschmidt <goldsimon>
Group administrator
Wed 17 Jun 2020 06:24:30 AM UTC, comment #4: 

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

Mate Varga <matevarga>
Mon 08 Jun 2020 09:17:13 AM UTC, comment #3: 

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

Mate Varga <matevarga>
Mon 08 Jun 2020 08:55:05 AM UTC, comment #2: 

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."

Mate Varga <matevarga>
Sun 07 Jun 2020 01:50:40 PM UTC, comment #1: 

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?

Erik Ekman <yarrick>
Group Member
Sun 31 May 2020 09:31:13 PM UTC, original submission:  

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.

Mate Varga <matevarga>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by yarrick (Posted a comment)
  • -email is unavailable- added by matevarga (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-31 matevarga Attached File- Added 0001-Does-not-add-an-empty-label-when-encountering-traili.patch, #49191

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code