This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] resolv: Support an exactly sized buffer in ns_name_pack [BZ #21359]
- From: fweimer at redhat dot com (Florian Weimer)
- To: libc-alpha at sourceware dot org
- Date: Fri, 07 Apr 2017 15:38:19 +0200
- Subject: [PATCH] resolv: Support an exactly sized buffer in ns_name_pack [BZ #21359]
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7B632C059750
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7B632C059750
2017-04-07 Florian Weimer <fweimer@redhat.com>
[BZ #21359]
* resolv/ns_name.c (ns_name_pack): Do not require an additional
byte in the destination buffer. Avoid out-of-bounds pointer
arithmetic.
* resolv/Makefile (tests): Add tst-ns_name_compress.
(tst-ns_name_compress): Link with -lresolv.
* resolv/tst-ns_name_compress.c: New file.
* resolv/tst-resolv-basic.c (LONG_NAME): Define.
(response): Recognize LONG_NAME.
(do_test): Add LONG_NAME tests.
* resolv/tst-ns_name.c (run_test_case): Fix expected data check
for ns_name_unpack. Add tests for ns_name_pton and
ns_name_compress.
diff --git a/resolv/Makefile b/resolv/Makefile
index 5a867b7..c69b24b 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -44,6 +44,7 @@ tests += \
tst-bug18665 \
tst-bug18665-tcp \
tst-ns_name \
+ tst-ns_name_compress \
tst-res_hconf_reorder \
tst-res_use_inet6 \
tst-resolv-basic \
@@ -140,6 +141,7 @@ $(objpfx)tst-resolv-canonname: \
$(objpfx)tst-ns_name: $(objpfx)libresolv.so
$(objpfx)tst-ns_name.out: tst-ns_name.data
+$(objpfx)tst-ns_name_compress: $(objpfx)libresolv.so
# This test case uses the deprecated RES_USE_INET6 resolver option.
diff --git a/resolv/ns_name.c b/resolv/ns_name.c
index 0d76fe5..08a75e2 100644
--- a/resolv/ns_name.c
+++ b/resolv/ns_name.c
@@ -475,7 +475,7 @@ ns_name_pack(const u_char *src, u_char *dst, int dstsiz,
goto cleanup;
}
n = labellen(srcp);
- if (dstp + 1 + n >= eob) {
+ if (n + 1 > eob - dstp) {
goto cleanup;
}
memcpy(dstp, srcp, n + 1);
diff --git a/resolv/tst-ns_name.c b/resolv/tst-ns_name.c
index 66d9a66..65eea4c 100644
--- a/resolv/tst-ns_name.c
+++ b/resolv/tst-ns_name.c
@@ -195,6 +195,7 @@ print_hex (const char *label, struct buffer buffer)
static void
run_test_case (struct test_case *t)
{
+ /* Test ns_name_unpack. */
unsigned char *unpacked = xmalloc (NS_MAXCDNAME);
int consumed = ns_name_unpack
(t->input.data, t->input.data + t->input.length,
@@ -211,16 +212,19 @@ run_test_case (struct test_case *t)
}
if (consumed != -1)
{
- if (memcmp (unpacked, t->unpack_output.data, consumed) != 0)
+ if (memcmp (unpacked, t->unpack_output.data,
+ t->unpack_output.length) != 0)
{
support_record_failure ();
printf ("%s:%zu: error: wrong data from ns_name_unpack\n",
t->path, t->lineno);
print_hex ("expected:", t->unpack_output);
- print_hex ("actual: ", (struct buffer) { unpacked, consumed });
+ print_hex ("actual: ",
+ (struct buffer) { unpacked, t->unpack_output.length });
return;
}
+ /* Test ns_name_ntop. */
char *text = xmalloc (NS_MAXDNAME);
int ret = ns_name_ntop (unpacked, text, NS_MAXDNAME);
if (ret != t->ntop_result)
@@ -243,6 +247,137 @@ run_test_case (struct test_case *t)
printf (" actual: \"%s\"\n", text);
return;
}
+
+ /* Test ns_name_pton. Unpacking does not check the
+ NS_MAXCDNAME limit, but packing does, so we need to
+ adjust the expected result. */
+ int expected;
+ if (t->unpack_output.length > NS_MAXCDNAME)
+ expected = -1;
+ else if (strcmp (text, ".") == 0)
+ /* The root domain is fully qualified. */
+ expected = 1;
+ else
+ /* The domain name is never fully qualified. */
+ expected = 0;
+ unsigned char *repacked = xmalloc (NS_MAXCDNAME);
+ ret = ns_name_pton (text, repacked, NS_MAXCDNAME);
+ if (ret != expected)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong result from ns_name_pton\n"
+ " expected: %d\n"
+ " actual: %d\n",
+ t->path, t->lineno, expected, ret);
+ return;
+ }
+ if (ret >= 0
+ && memcmp (repacked, unpacked, t->unpack_output.length) != 0)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong data from ns_name_pton\n",
+ t->path, t->lineno);
+ print_hex ("expected:", t->unpack_output);
+ print_hex ("actual: ",
+ (struct buffer) { repacked, t->unpack_output.length });
+ return;
+ }
+
+ /* Test ns_name_compress, no compression case. */
+ if (t->unpack_output.length > NS_MAXCDNAME)
+ expected = -1;
+ else
+ expected = t->unpack_output.length;
+ memset (repacked, '$', NS_MAXCDNAME);
+ {
+ enum { ptr_count = 5 };
+ const unsigned char *dnptrs[ptr_count] = { repacked, };
+ ret = ns_name_compress (text, repacked, NS_MAXCDNAME,
+ dnptrs, dnptrs + ptr_count);
+ if (ret != expected)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong result from ns_name_compress\n"
+ " expected: %d\n"
+ " actual: %d\n",
+ t->path, t->lineno, expected, ret);
+ return;
+ }
+ if (ret < 0)
+ {
+ TEST_VERIFY (dnptrs[0] == repacked);
+ TEST_VERIFY (dnptrs[1] == NULL);
+ }
+ else
+ {
+ if (memcmp (repacked, unpacked, t->unpack_output.length) != 0)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong data from ns_name_compress\n",
+ t->path, t->lineno);
+ print_hex ("expected:", t->unpack_output);
+ print_hex ("actual: ", (struct buffer) { repacked, ret });
+ return;
+ }
+ TEST_VERIFY (dnptrs[0] == repacked);
+ if (unpacked[0] == '\0')
+ /* The root domain is not a compression target. */
+ TEST_VERIFY (dnptrs[1] == NULL);
+ else
+ {
+ TEST_VERIFY (dnptrs[1] == repacked);
+ TEST_VERIFY (dnptrs[2] == NULL);
+ }
+ }
+ }
+
+ /* Test ns_name_compress, full compression case. Skip this
+ test for invalid names and the root domain. */
+ if (expected >= 0 && unpacked[0] != '\0')
+ {
+ /* The destination buffer needs additional room for the
+ offset, the initial name, and the compression
+ reference. */
+ enum { name_offset = 259 };
+ size_t target_offset = name_offset + t->unpack_output.length;
+ size_t repacked_size = target_offset + 2;
+ repacked = xrealloc (repacked, repacked_size);
+ memset (repacked, '@', repacked_size);
+ memcpy (repacked + name_offset,
+ t->unpack_output.data, t->unpack_output.length);
+ enum { ptr_count = 5 };
+ const unsigned char *dnptrs[ptr_count]
+ = { repacked, repacked + name_offset, };
+ ret = ns_name_compress
+ (text, repacked + target_offset, NS_MAXCDNAME,
+ dnptrs, dnptrs + ptr_count);
+ if (ret != 2)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong result from ns_name_compress"
+ " (2)\n"
+ " expected: 2\n"
+ " actual: %d\n",
+ t->path, t->lineno, ret);
+ return;
+ }
+ if (memcmp (repacked + target_offset, "\xc1\x03", 2) != 0)
+ {
+ support_record_failure ();
+ printf ("%s:%zu: error: wrong data from ns_name_compress"
+ " (2)\n"
+ " expected: C103\n",
+ t->path, t->lineno);
+ print_hex ("actual: ",
+ (struct buffer) { repacked + target_offset, ret });
+ return;
+ }
+ TEST_VERIFY (dnptrs[0] == repacked);
+ TEST_VERIFY (dnptrs[1] == repacked + name_offset);
+ TEST_VERIFY (dnptrs[2] == NULL);
+ }
+
+ free (repacked);
}
free (text);
}
diff --git a/resolv/tst-ns_name_compress.c b/resolv/tst-ns_name_compress.c
new file mode 100644
index 0000000..70b4328
--- /dev/null
+++ b/resolv/tst-ns_name_compress.c
@@ -0,0 +1,74 @@
+/* Test ns_name_compress corner cases.
+ Copyright (C) 2017 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
+ <http://www.gnu.org/licenses/>. */
+
+#include <resolv.h>
+#include <support/support.h>
+#include <support/check.h>
+
+/* Check that we can process names which fit into the destination
+ buffer exactly. See bug 21359. */
+static void
+test_exact_fit (const char *name, size_t length)
+{
+ unsigned char *buf = xmalloc (length + 1);
+ memset (buf, '$', length + 1);
+ enum { ptr_count = 5 };
+ const unsigned char *dnptrs[ptr_count] = { buf, };
+ int ret = ns_name_compress (name, buf, length,
+ dnptrs, dnptrs + ptr_count);
+ if (ret < 0)
+ {
+ support_record_failure ();
+ printf ("error: ns_name_compress for %s/%zu failed\n", name, length);
+ return;
+ }
+ if ((size_t) ret != length)
+ {
+ support_record_failure ();
+ printf ("error: ns_name_compress for %s/%zu result mismatch: %d\n",
+ name, length, ret);
+ }
+ if (buf[length] != '$')
+ {
+ support_record_failure ();
+ printf ("error: ns_name_compress for %s/%zu padding write\n",
+ name, length);
+ }
+}
+
+static int
+do_test (void)
+{
+ test_exact_fit ("abc", 5);
+ test_exact_fit ("abc.", 5);
+ {
+ char long_name[]
+ = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.";
+ TEST_VERIFY (strlen (long_name) == NS_MAXCDNAME - 1);
+ test_exact_fit (long_name, NS_MAXCDNAME);
+ long_name[sizeof (long_name) - 1] = '\0';
+ test_exact_fit (long_name, NS_MAXCDNAME);
+ }
+ return 0;
+}
+
+#include <support/test-driver.c>
+
diff --git a/resolv/tst-resolv-basic.c b/resolv/tst-resolv-basic.c
index 94b1631..f2b1fc7 100644
--- a/resolv/tst-resolv-basic.c
+++ b/resolv/tst-resolv-basic.c
@@ -25,6 +25,12 @@
#include <support/resolv_test.h>
#include <support/support.h>
+#define LONG_NAME \
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaax." \
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay." \
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaz." \
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaat"
+
static void
response (const struct resolv_response_context *ctx,
struct resolv_response_builder *b,
@@ -43,13 +49,15 @@ response (const struct resolv_response_context *ctx,
qname_compare = qname + 2;
else
qname_compare = qname;
- enum {www, alias, nxdomain} requested_qname;
+ enum {www, alias, nxdomain, long_name} requested_qname;
if (strcmp (qname_compare, "www.example") == 0)
requested_qname = www;
else if (strcmp (qname_compare, "alias.example") == 0)
requested_qname = alias;
else if (strcmp (qname_compare, "nxdomain.example") == 0)
requested_qname = nxdomain;
+ else if (strcmp (qname_compare, LONG_NAME) == 0)
+ requested_qname = long_name;
else
{
support_record_failure ();
@@ -69,6 +77,7 @@ response (const struct resolv_response_context *ctx,
switch (requested_qname)
{
case www:
+ case long_name:
resolv_response_open_record (b, qname, qclass, qtype, 0);
break;
case alias:
@@ -209,6 +218,10 @@ do_test (void)
"name: www.example\n"
"alias: alias.example\n"
"address: 2001:db8::2\n");
+ check_h (LONG_NAME, AF_INET,
+ "name: " LONG_NAME "\n"
+ "address: 192.0.2.20\n");
+
check_ai ("www.example", "80", AF_UNSPEC,
"address: STREAM/TCP 192.0.2.17 80\n"
"address: DGRAM/UDP 192.0.2.17 80\n"
@@ -223,6 +236,13 @@ do_test (void)
"address: STREAM/TCP 2001:db8::2 80\n"
"address: DGRAM/UDP 2001:db8::2 80\n"
"address: RAW/IP 2001:db8::2 80\n");
+ check_ai (LONG_NAME, "80", AF_UNSPEC,
+ "address: STREAM/TCP 192.0.2.20 80\n"
+ "address: DGRAM/UDP 192.0.2.20 80\n"
+ "address: RAW/IP 192.0.2.20 80\n"
+ "address: STREAM/TCP 2001:db8::4 80\n"
+ "address: DGRAM/UDP 2001:db8::4 80\n"
+ "address: RAW/IP 2001:db8::4 80\n");
check_ai ("www.example", "80", AF_INET,
"address: STREAM/TCP 192.0.2.17 80\n"
"address: DGRAM/UDP 192.0.2.17 80\n"
@@ -231,6 +251,10 @@ do_test (void)
"address: STREAM/TCP 192.0.2.18 80\n"
"address: DGRAM/UDP 192.0.2.18 80\n"
"address: RAW/IP 192.0.2.18 80\n");
+ check_ai (LONG_NAME, "80", AF_INET,
+ "address: STREAM/TCP 192.0.2.20 80\n"
+ "address: DGRAM/UDP 192.0.2.20 80\n"
+ "address: RAW/IP 192.0.2.20 80\n");
check_ai ("www.example", "80", AF_INET6,
"address: STREAM/TCP 2001:db8::1 80\n"
"address: DGRAM/UDP 2001:db8::1 80\n"
@@ -239,6 +263,10 @@ do_test (void)
"address: STREAM/TCP 2001:db8::2 80\n"
"address: DGRAM/UDP 2001:db8::2 80\n"
"address: RAW/IP 2001:db8::2 80\n");
+ check_ai (LONG_NAME, "80", AF_INET6,
+ "address: STREAM/TCP 2001:db8::4 80\n"
+ "address: DGRAM/UDP 2001:db8::4 80\n"
+ "address: RAW/IP 2001:db8::4 80\n");
check_h ("t.www.example", AF_INET,
"name: t.www.example\n"