Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release proposal: v4.6.1 #9153

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
deps: avoid single-byte buffer overwrite
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: #9037
PR-URL: #8849
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
  • Loading branch information
bagder authored and rvagg committed Oct 18, 2016
commit f3c63e7ccf92cb5050a535efc5be2333a53c09c0
84 changes: 39 additions & 45 deletions deps/cares/src/ares_create_query.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,57 +85,31 @@
*/

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 = malloc(*buflen);
if (!*buf)
return ARES_ENOMEM;
len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ +
(max_udp_size ? EDNSFIXEDSZ : 0);
buf = 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);
Expand All @@ -159,8 +133,10 @@ int ares_create_query(const char *name, int dnsclass, int type,
q += HFIXEDSZ;
while (*name)
{
if (*name == '.')
if (*name == '.') {
free (buf);
return ARES_EBADNAME;
}

/* Count the number of bytes in this label. */
len = 0;
Expand All @@ -170,8 +146,10 @@ int ares_create_query(const char *name, int dnsclass, int type,
p++;
len++;
}
if (len > MAXLABEL)
if (len > MAXLABEL) {
free (buf);
return ARES_EBADNAME;
}

/* Encode the length and copy the data. */
*q++ = (unsigned char)len;
Expand All @@ -195,14 +173,30 @@ int ares_create_query(const char *name, int dnsclass, int type,
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;
}