-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
5 changed files
with
213 additions
and
35 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,4 +5,6 @@ _site | |
c-ares.git | ||
roffit.git | ||
docs | ||
security.md | ||
license.md | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
From 115fe381c75147352d7a8d21aa3ffb85ca367219 Mon Sep 17 00:00:00 2001 | ||
From: Daniel Stenberg <[email protected]> | ||
Date: Fri, 23 Sep 2016 14:44:11 +0200 | ||
Subject: [PATCH] ares_create_query: avoid single-byte buffer overwrite | ||
|
||
... when the name ends with an escaped dot. | ||
|
||
CVE-2016-5180 | ||
|
||
Bug: https://c-ares.haxx.se/adv_20160929.html | ||
--- | ||
ares_create_query.c | 84 +++++++++++++++++++++++++---------------------------- | ||
1 file changed, 39 insertions(+), 45 deletions(-) | ||
|
||
diff --git a/ares_create_query.c b/ares_create_query.c | ||
index a34dda7..7f4c52d 100644 | ||
--- a/ares_create_query.c | ||
+++ b/ares_create_query.c | ||
@@ -83,61 +83,35 @@ | ||
* label. The list is terminated by a label of length zero (which can | ||
* be thought of as the root domain). | ||
*/ | ||
|
||
int ares_create_query(const char *name, int dnsclass, int type, | ||
- unsigned short id, int rd, unsigned char **buf, | ||
- int *buflen, int max_udp_size) | ||
+ unsigned short id, int rd, unsigned char **bufp, | ||
+ int *buflenp, int max_udp_size) | ||
{ | ||
- int len; | ||
+ size_t len; | ||
unsigned char *q; | ||
const char *p; | ||
+ size_t buflen; | ||
+ unsigned char *buf; | ||
|
||
/* Set our results early, in case we bail out early with an error. */ | ||
- *buflen = 0; | ||
- *buf = NULL; | ||
+ *buflenp = 0; | ||
+ *bufp = NULL; | ||
|
||
- /* Compute the length of the encoded name so we can check buflen. | ||
- * Start counting at 1 for the zero-length label at the end. */ | ||
- len = 1; | ||
- for (p = name; *p; p++) | ||
- { | ||
- if (*p == '\\' && *(p + 1) != 0) | ||
- p++; | ||
- len++; | ||
- } | ||
- /* If there are n periods in the name, there are n + 1 labels, and | ||
- * thus n + 1 length fields, unless the name is empty or ends with a | ||
- * period. So add 1 unless name is empty or ends with a period. | ||
+ /* Allocate a memory area for the maximum size this packet might need. +2 | ||
+ * is for the length byte and zero termination if no dots or ecscaping is | ||
+ * used. | ||
*/ | ||
- if (*name && *(p - 1) != '.') | ||
- len++; | ||
- | ||
- /* Immediately reject names that are longer than the maximum of 255 | ||
- * bytes that's specified in RFC 1035 ("To simplify implementations, | ||
- * the total length of a domain name (i.e., label octets and label | ||
- * length octets) is restricted to 255 octets or less."). We aren't | ||
- * doing this just to be a stickler about RFCs. For names that are | ||
- * too long, 'dnscache' closes its TCP connection to us immediately | ||
- * (when using TCP) and ignores the request when using UDP, and | ||
- * BIND's named returns ServFail (TCP or UDP). Sending a request | ||
- * that we know will cause 'dnscache' to close the TCP connection is | ||
- * painful, since that makes any other outstanding requests on that | ||
- * connection fail. And sending a UDP request that we know | ||
- * 'dnscache' will ignore is bad because resources will be tied up | ||
- * until we time-out the request. | ||
- */ | ||
- if (len > MAXCDNAME) | ||
- return ARES_EBADNAME; | ||
- | ||
- *buflen = len + HFIXEDSZ + QFIXEDSZ + (max_udp_size ? EDNSFIXEDSZ : 0); | ||
- *buf = ares_malloc(*buflen); | ||
- if (!*buf) | ||
- return ARES_ENOMEM; | ||
+ len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ + | ||
+ (max_udp_size ? EDNSFIXEDSZ : 0); | ||
+ buf = ares_malloc(len); | ||
+ if (!buf) | ||
+ return ARES_ENOMEM; | ||
|
||
/* Set up the header. */ | ||
- q = *buf; | ||
+ q = buf; | ||
memset(q, 0, HFIXEDSZ); | ||
DNS_HEADER_SET_QID(q, id); | ||
DNS_HEADER_SET_OPCODE(q, QUERY); | ||
if (rd) { | ||
DNS_HEADER_SET_RD(q, 1); | ||
@@ -157,23 +131,27 @@ int ares_create_query(const char *name, int dnsclass, int type, | ||
|
||
/* Start writing out the name after the header. */ | ||
q += HFIXEDSZ; | ||
while (*name) | ||
{ | ||
- if (*name == '.') | ||
+ if (*name == '.') { | ||
+ free (buf); | ||
return ARES_EBADNAME; | ||
+ } | ||
|
||
/* Count the number of bytes in this label. */ | ||
len = 0; | ||
for (p = name; *p && *p != '.'; p++) | ||
{ | ||
if (*p == '\\' && *(p + 1) != 0) | ||
p++; | ||
len++; | ||
} | ||
- if (len > MAXLABEL) | ||
+ if (len > MAXLABEL) { | ||
+ free (buf); | ||
return ARES_EBADNAME; | ||
+ } | ||
|
||
/* Encode the length and copy the data. */ | ||
*q++ = (unsigned char)len; | ||
for (p = name; *p && *p != '.'; p++) | ||
{ | ||
@@ -193,16 +171,32 @@ int ares_create_query(const char *name, int dnsclass, int type, | ||
|
||
/* Finish off the question with the type and class. */ | ||
DNS_QUESTION_SET_TYPE(q, type); | ||
DNS_QUESTION_SET_CLASS(q, dnsclass); | ||
|
||
+ q += QFIXEDSZ; | ||
if (max_udp_size) | ||
{ | ||
- q += QFIXEDSZ; | ||
memset(q, 0, EDNSFIXEDSZ); | ||
q++; | ||
DNS_RR_SET_TYPE(q, T_OPT); | ||
DNS_RR_SET_CLASS(q, max_udp_size); | ||
+ q += (EDNSFIXEDSZ-1); | ||
+ } | ||
+ buflen = (q - buf); | ||
+ | ||
+ /* Reject names that are longer than the maximum of 255 bytes that's | ||
+ * specified in RFC 1035 ("To simplify implementations, the total length of | ||
+ * a domain name (i.e., label octets and label length octets) is restricted | ||
+ * to 255 octets or less."). */ | ||
+ if (buflen > (MAXCDNAME + HFIXEDSZ + QFIXEDSZ + | ||
+ (max_udp_size ? EDNSFIXEDSZ : 0))) { | ||
+ free (buf); | ||
+ return ARES_EBADNAME; | ||
} | ||
|
||
+ /* we know this fits in an int at this point */ | ||
+ *buflenp = (int) buflen; | ||
+ *bufp = buf; | ||
+ | ||
return ARES_SUCCESS; | ||
} | ||
-- | ||
2.9.3 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
From e1f43d4d7e89ef8db479d6efd0389c6b6ee1d116 Mon Sep 17 00:00:00 2001 | ||
From: David Drysdale <[email protected]> | ||
Date: Mon, 22 May 2017 10:54:10 +0100 | ||
Subject: [PATCH 5/5] ares_parse_naptr_reply: check sufficient data | ||
|
||
Check that there is enough data for the required elements | ||
of an NAPTR record (2 int16, 3 bytes for string lengths) | ||
before processing a record. | ||
--- | ||
ares_parse_naptr_reply.c | 7 ++++++- | ||
1 file changed, 6 insertions(+), 1 deletion(-) | ||
|
||
diff --git a/ares_parse_naptr_reply.c b/ares_parse_naptr_reply.c | ||
index 11634df9847c..717d35577811 100644 | ||
--- a/ares_parse_naptr_reply.c | ||
+++ b/ares_parse_naptr_reply.c | ||
@@ -110,6 +110,12 @@ ares_parse_naptr_reply (const unsigned char *abuf, int alen, | ||
status = ARES_EBADRESP; | ||
break; | ||
} | ||
+ /* RR must contain at least 7 bytes = 2 x int16 + 3 x name */ | ||
+ if (rr_len < 7) | ||
+ { | ||
+ status = ARES_EBADRESP; | ||
+ break; | ||
+ } | ||
|
||
/* Check if we are really looking at a NAPTR record */ | ||
if (rr_class == C_IN && rr_type == T_NAPTR) | ||
@@ -185,4 +191,3 @@ ares_parse_naptr_reply (const unsigned char *abuf, int alen, | ||
|
||
return ARES_SUCCESS; | ||
} | ||
- | ||
-- | ||
2.13.0.303.g4ebf302169-goog | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.