423287 - net_IsValidHostName() should perform length-based sanity checks per RFC 1035

Status: NEW

(Core :: Networking, defect, P3)

Description

[Neil Harris]

At the moment, net_IsValidHostName() only makes character-set-based sanity checks for names other than IPv6 addresses, and ignores the internal structure of the name and the lengths of its components. Thus,  net_IsValidHostName() currently regards any arbitrarily long string of any character or characters entirely within the whitelist as a valid hostname, including degenerate cases such as the null string, or a multi-megabyte string consisting entirely of "." characters. 

This potentially allows malformed hostnames to be accepted within the system, or passed outside the system, possibly leading to errors unless some other part of the code catches these names either before or after the net_IsValidHostName() check. At the moment, I cannot find any specific cases which provoke errors, but I believe that net_IsValidHostName() should catch them anyway as part of a policy of defensive programming.

To fix this, net_IsValidHostName() should, as its name suggests, enforce the limits on hostnames/domain names defined by RFC 1035, namely
* Labels must be 63 characters or less.
* Names must be 255 octets or less
as well as the requirement (derived from the given BNF grammar) that names should include at least one label, and labels should have at least one character. (However, the commonly-used real-world special case of names which end in a dot should not cause an error.)

Since the current code iterates over every character in the entire name anyway, this check would require only a little extra counting during the traversal, with bounds checks needed only at label transitions, and impact on performance would thus be minimal.

Comment 1

[benc]

I agree with most points, but:

it sounds like you want the hostname to be valid FQDN. Shouldn't you name the function to say that?

I thought the maxlength was 252..

There are also valid character restrictions as well.

Comment 2

[Neil Harris]

From RFC 1035: 

"2.3.4. Size limits

Various objects and parameters in the DNS have size limits.  They are
listed below.  Some could be easily changed, others are more
fundamental.

labels          63 octets or less

names           255 octets or less

TTL             positive values of a signed 32 bit number.

UDP messages    512 octets or less"

Comment 3

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 4

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 5

[Valentin Gosu [:valentin] (he/him)]

https://searchfox.org/mozilla-central/rev/cee2c396081d950f9e3401113fb179999e404ab8/netwerk/base/nsURLHelper.cpp#870-873

bool net_IsValidHostName(const nsACString& host) {
  // The host name is limited to 253 ascii characters.
  if (host.Length() > 253) {
    return false;

We now check the max length of the host, but not that individual labels are 63 characters or less.