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

[Carlos O'Donell]

On 3/20/26 4:39 PM, Florian Weimer wrote:
> * 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.

I've split the the commit in two to cover one CVE per commit.

Please note that after review of the last few CVEs the glibc security team
updated the team process on 2026-01-15 to limit backport work to the most
recent released branch e.g. release/2.43/master.

See 9.2 in https://sourceware.org/glibc/wiki/CNA/Response

We leave it up to the interested developers to do further backports.

With increased security reports arriving the glibc security team wants to
make sure we have capacity to handle new reports.

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

Correct, both need free (tail) given the use of %m. Fixed in both.

Added a FAIL_EXIT to this test for invalid qname to match similar tests.

>> +  TEST_VERIFY (count <= 15);
> 
>> +  TEST_VERIFY (count < 16);
> 
> These checks should use array_length (test_items).
> 
> This also applies to the second test case.

Agreed. Fixed in both.

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

Good point. I've removed the TEST_COMPARE_STRING since it's redundant. Fixed.

I've changed the printf to "error: unexpected success: %s\n" in both tests. Fixed.

>> +    /* 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.

Windows DNS claims to support UTF-8 characters in names:
https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou

So I wanted to include a test that showed UTF-8 as invalid since our
implementation doesn't currently support it.

My preference is to leave them, and if they are difficult to backport we
can revisit a simplified or alternate test that encodes them in a
different way?

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

Fixed.

I'll post v2.

-- 
Cheers,
Carlos.