[PATCH v2 8/8] resolv: Add test case tst-ns_sprintrr (bug 34033, bug 34069)

[Florian Weimer]

* Adhemerval Zanella Netto:

> On 16/06/26 13:32, Florian Weimer wrote:
>> This test case covers both input buffer overreads and output buffer
>> overflows.  It should systematically cover these issues.
>> 
>> I used code auto-generation for updating the test expectations for
>> truncated RDATA in TXT, ISDN records, after writing the rest
>> of the test by hand.
>> 
>> Assisted-by: LLM
>
> LGTM, some suggestions below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
>> ---
>>  resolv/Makefile          |   2 +
>>  resolv/tst-ns_sprintrr.c | 328 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 330 insertions(+)
>>  create mode 100644 resolv/tst-ns_sprintrr.c
>> 
>> diff --git a/resolv/Makefile b/resolv/Makefile
>> index 68b3a4dbf3..02cc751732 100644
>> --- a/resolv/Makefile
>> +++ b/resolv/Makefile
>> @@ -108,6 +108,7 @@ tests += \
>>    tst-ns_name \
>>    tst-ns_name_compress \
>>    tst-ns_name_pton \
>> +  tst-ns_sprintrr \
>>    tst-res_hconf_reorder \
>>    tst-res_hnok \
>>    tst-resolv-aliases \
>
> Ok.
>
>> @@ -341,5 +342,6 @@ $(objpfx)tst-ns_name: $(objpfx)libresolv.so
>>  $(objpfx)tst-ns_name.out: tst-ns_name.data
>>  $(objpfx)tst-ns_name_compress: $(objpfx)libresolv.so
>>  $(objpfx)tst-ns_name_pton: $(objpfx)libresolv.so
>> +$(objpfx)tst-ns_sprintrr: $(objpfx)libresolv.so
>>  $(objpfx)tst-res_hnok: $(objpfx)libresolv.so
>>  $(objpfx)tst-p_secstodate: $(objpfx)libresolv.so
>> diff --git a/resolv/tst-ns_sprintrr.c b/resolv/tst-ns_sprintrr.c
>> new file mode 100644
>> index 0000000000..9083d28d2d
>> --- /dev/null
>> +++ b/resolv/tst-ns_sprintrr.c
>> @@ -0,0 +1,328 @@
>> +/* Tests for the ns_sprintrr function.
>> +   Copyright (C) 2026 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#include <arpa/nameser.h>
>> +
>> +#include <alloc_buffer.h>
>> +#include <arpa/inet.h>
>> +#include <libc-diag.h>
>> +#include <stdbool.h>
>> +#include <string.h>
>> +#include <support/check.h>
>> +#include <support/next_to_fault.h>
>> +
>> +#include <stdio.h>
>> +
>> +/* Regions that test_one_record uses for input and output.  */
>> +static struct support_next_to_fault ntf_in;
>> +static struct support_next_to_fault ntf_out;
>> +
>> +/* This is used by test_one_record to construct the packet.   */
>> +static const char packet_prefix[] =
>> +  /* DNS response with one question, one answer record.  */
>> +  "AA\x81\x80\0\1\0\1\0\0\0\0"
>> +  /* Question: www.example.org/IN/ANY.  */
>> +  "\3www\7example\3org\0\0\xff\0\1"
>> +  /* Response: compression reference.  */
>> +  "\xc0\x0c";
>> +
>> +/* Use ns_sprintrr to format a DNS record (starting with
>> +   packet_prefix) of type RTYPE, with a record payload of RDATALEN
>> +   bytes starting at RDATA.  Check successful formatting against
>> +   EXPECTED.  Try various truncated input and output buffers to catch
>> +   overreads and buffer overflows, using ntf_in and ntf_out above.  */
>> +static void
>> +test_one_record (uint16_t rtype, const char *rdata, size_t rdatalen,
>> +                 const char *expected)
>> +{
>> +  struct rr_header
>> +  {
>> +    uint16_t typ;
>> +    uint16_t cls;
>> +    uint32_t ttl;
>> +    uint16_t rdatalen;
>> +    uint16_t pad;
>> +  } hdr =
>> +    {
>> +      .typ = htons (rtype),
>> +      .cls = htons (ns_c_in),
>> +      .ttl = htonl (86400),     /* One day.  */
>> +      .rdatalen = htons (rdatalen),
>> +    };
>> +  size_t hdrlen = offsetof (struct rr_header, pad);
>> +  TEST_COMPARE (hdrlen, 10);
>
> I haven't checked this on all supported ABIs, but this seems to be true for
> all supported ABIs.  I think you can make it a _Static_assert instead.

It has to be 10.  But hdrlen is not a constant expression, so I would
have to repeat it.  I'm going to change it to enum constant.

>> +  /* Construct the packet from packet_prefix, hdr, and rdata.  */
>> +  char packet[512];
>
> I think you can make this unsigned and remove the cast below.

Yeah, unsigned char * leads to fewer casts in the end.

>> +  bool success = false;
>> +  for (size_t result_size = 1; result_size <= max_result_size; ++result_size)
>> +    {
>> +      char *result_start = ntf_out.buffer + ntf_out.length - result_size;
>> +      memset (result_start, 'X', result_size);
>> +
>> +      /* ns_sprintrr was deprecated in 2.34.  */
>> +      DIAG_PUSH_NEEDS_COMMENT;
>> +      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
>
> Maybe use 12.0, since it is the current minimal support version
> anyway.

Not sure what the rules for permanent exemptions are.  Should I use
16.1?

Thanks,
Florian