From patchwork Fri Apr 7 13:38:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 748286 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3w00yy3MXyz9s7s for ; Fri, 7 Apr 2017 23:38:34 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="uV4eu0KH"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= PWtka3q1dfyJNT6+6+5mU90vWbS0q7I94TZtYLGYr3oEz1HI0dNGWCVpK9sUXCUk o7Yo0fqTsbu9415CaW+TUOdj8JJZTHO8YYbOqRSCrqOSmg/kXaUs7b1kEdvd8Z6f bVReNUOWHAapssPIhu8G8hVgPzLOgPNfHkTziLeIU7s= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=T2HgWr AlL2OhQf33nZQ5L962eBo=; b=uV4eu0KHsuZwSsqDLv8fRUmDkDfwS/Lf5eKSuW 8bwqfNvsCGBiZlAPPdxpbXxdRUs+SWwc04Se2SoxUCCeIzXI2M+0h04DWCmNtr4Y vowBChbe6PJWBsbbVrl3Rivw61gh2u3BucODZlDYX65s+WY87Rhs+bstOCchKmGX fkT5Q= Received: (qmail 46806 invoked by alias); 7 Apr 2017 13:38:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 46349 invoked by uid 89); 7 Apr 2017 13:38:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=23613, 2512, STREAM X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7B632C059750 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@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7B632C059750 Date: Fri, 07 Apr 2017 15:38:19 +0200 To: libc-alpha@sourceware.org Subject: [PATCH] resolv: Support an exactly sized buffer in ns_name_pack [BZ #21359] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20170407133819.2656453F954E4@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) 2017-04-07 Florian Weimer [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 + . */ + +#include +#include +#include + +/* 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 + 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 #include +#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"