[PATCH] resolv: Count records, and check hostname (CVE-2026-4437, CVE-2026-4438)

[Florian Weimer]

* Carlos O'Donell:

> On 3/20/26 4:11 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> The answer section boundary was previously ignored, and the code in
>>> getanswer_ptr would iterate past the last resource record, but not
>>> beyond the end of the returned data.  This could lead to subsequent data
>>> being interpreted as answer records, thus violating the DNS
>>> specification.  Such resource records could be maliciously crafted and
>>> hidden from other tooling, but processed by the glibc stub resolver and
>>> acted upon by the application.  While we trust the data returned by the
>>> configured recursive resolvers, we should not trust its format and
>>> should validate it as required.  It is a security issue to incorrectly
>>> process the DNS protocol.
>>>
>>> The processed hostname in getanswer_ptr should be correctly checked to
>>> avoid invalid characters from being allowed, including shell
>>> metacharacters. It is a security issue to fail to check the returned
>>> hostname for validity.
>>>
>>> These two issues are considered distinct CVEs, but are fixed in one
>>> commit to make the update process easier, given that they change the
>>> same file and function.
>>>
>>> Regression tests are added for invalid metacharacters and response
>>> section crossing.
>>>
>>> No regressions on x86_64-linux-gnu.
>>> ---
>>>   resolv/Makefile                 |   7 +
>>>   resolv/nss_dns/dns-host.c       |   4 +-
>>>   resolv/tst-resolv-dns-section.c | 161 ++++++++++++++++++++
>>>   resolv/tst-resolv-invalid-ptr.c | 258 ++++++++++++++++++++++++++++++++
>>>   4 files changed, 428 insertions(+), 2 deletions(-)
>>>   create mode 100644 resolv/tst-resolv-dns-section.c
>>>   create mode 100644 resolv/tst-resolv-invalid-ptr.c

>> I don't quite understand why this is a single commit for two bugs
>> that have very different impact, fixes, and even separate test cases.
>
> The justification is in the commit log? :-)
> ~~~
> These two issues are considered distinct CVEs, but are fixed in one
> commit to make the update process easier, given that they change the
> same file and function.
> ~~~

If that's the only justification, I don't think we should put both into
one patch.

Couple of review notes:

> +  if (strstr (qname, "in-addr.arpa") != NULL
> +      && sscanf (qname, "%u.%ms", &count, &tail) == 2)
> +    TEST_COMPARE_STRING (tail, "0.168.192.in-addr.arpa");
> +  else if (sscanf (qname, "%x.%ms", &count, &tail) == 3)
> +    {
> +    TEST_COMPARE_STRING (tail, "\
> +0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa");
> +    }

It seems that tail is never freed?

This also applies to the second test case.

> +  TEST_VERIFY (count <= 15);

> +  TEST_VERIFY (count < 16);

These checks should use array_length (test_items).

This also applies to the second test case.

> +  if (answer != NULL)
> +    {
> +      printf ("INFO: %s\n", support_format_hostent (answer));
> +      TEST_COMPARE_STRING (answer->h_name, "test.ptr.example.net");
> +    }

This is after the failure, and the information is already included in
the support_format_hostent output, so the TEST_COMPARE_STRING seems
quite redundant?  And printf could say "error: unexpected success: %s\n".
The last part also applies to the second test.

> +    /* Test for invalid UTF-8 characters (2-byte, 4-byte, 6-byte).  */
> +    { "Invalid use of UTF-8 (2-byte, U+00C0-U+00C2)",
> +      "ÁÂÃ.test.ptr.example", NO_RECOVERY, true },
> +    { "Invalid use of UTF-8 (4-byte, U+0750-U+0752)",
> +      "ݐݑݒ.test.ptr.example", NO_RECOVERY, true },
> +     { "Invalid use of UTF-8 (6-byte, U+0904-U+0906)",
> +      "ऄअआ.test.ptr.example", NO_RECOVERY, true },

The use of UTF-8 might be tricky to backport for some environments.

> +      TEST_VERIFY (answer != NULL);
> +      /* And it must match what we provided.  */
> +      if (answer != NULL)
> +        TEST_COMPARE_STRING (answer->h_name, test_items[count].answer);

TEST_COMPARE_STRING already handles NULL, so the extra checks aren't
necessary.

Thanks,
Florian