Skip to content

Commit c5b095e

Browse files
bagderrvagg
authored andcommitted
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>
1 parent c3f2f02 commit c5b095e

File tree

1 file changed

+39
-45
lines changed

1 file changed

+39
-45
lines changed

deps/cares/src/ares_create_query.c

+39-45
Original file line numberDiff line numberDiff line change
@@ -85,57 +85,31 @@
8585
*/
8686

8787
int ares_create_query(const char *name, int dnsclass, int type,
88-
unsigned short id, int rd, unsigned char **buf,
89-
int *buflen, int max_udp_size)
88+
unsigned short id, int rd, unsigned char **bufp,
89+
int *buflenp, int max_udp_size)
9090
{
91-
int len;
91+
size_t len;
9292
unsigned char *q;
9393
const char *p;
94+
size_t buflen;
95+
unsigned char *buf;
9496

9597
/* Set our results early, in case we bail out early with an error. */
96-
*buflen = 0;
97-
*buf = NULL;
98+
*buflenp = 0;
99+
*bufp = NULL;
98100

99-
/* Compute the length of the encoded name so we can check buflen.
100-
* Start counting at 1 for the zero-length label at the end. */
101-
len = 1;
102-
for (p = name; *p; p++)
103-
{
104-
if (*p == '\\' && *(p + 1) != 0)
105-
p++;
106-
len++;
107-
}
108-
/* If there are n periods in the name, there are n + 1 labels, and
109-
* thus n + 1 length fields, unless the name is empty or ends with a
110-
* period. So add 1 unless name is empty or ends with a period.
101+
/* Allocate a memory area for the maximum size this packet might need. +2
102+
* is for the length byte and zero termination if no dots or ecscaping is
103+
* used.
111104
*/
112-
if (*name && *(p - 1) != '.')
113-
len++;
114-
115-
/* Immediately reject names that are longer than the maximum of 255
116-
* bytes that's specified in RFC 1035 ("To simplify implementations,
117-
* the total length of a domain name (i.e., label octets and label
118-
* length octets) is restricted to 255 octets or less."). We aren't
119-
* doing this just to be a stickler about RFCs. For names that are
120-
* too long, 'dnscache' closes its TCP connection to us immediately
121-
* (when using TCP) and ignores the request when using UDP, and
122-
* BIND's named returns ServFail (TCP or UDP). Sending a request
123-
* that we know will cause 'dnscache' to close the TCP connection is
124-
* painful, since that makes any other outstanding requests on that
125-
* connection fail. And sending a UDP request that we know
126-
* 'dnscache' will ignore is bad because resources will be tied up
127-
* until we time-out the request.
128-
*/
129-
if (len > MAXCDNAME)
130-
return ARES_EBADNAME;
131-
132-
*buflen = len + HFIXEDSZ + QFIXEDSZ + (max_udp_size ? EDNSFIXEDSZ : 0);
133-
*buf = malloc(*buflen);
134-
if (!*buf)
135-
return ARES_ENOMEM;
105+
len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ +
106+
(max_udp_size ? EDNSFIXEDSZ : 0);
107+
buf = malloc(len);
108+
if (!buf)
109+
return ARES_ENOMEM;
136110

137111
/* Set up the header. */
138-
q = *buf;
112+
q = buf;
139113
memset(q, 0, HFIXEDSZ);
140114
DNS_HEADER_SET_QID(q, id);
141115
DNS_HEADER_SET_OPCODE(q, QUERY);
@@ -159,8 +133,10 @@ int ares_create_query(const char *name, int dnsclass, int type,
159133
q += HFIXEDSZ;
160134
while (*name)
161135
{
162-
if (*name == '.')
136+
if (*name == '.') {
137+
free (buf);
163138
return ARES_EBADNAME;
139+
}
164140

165141
/* Count the number of bytes in this label. */
166142
len = 0;
@@ -170,8 +146,10 @@ int ares_create_query(const char *name, int dnsclass, int type,
170146
p++;
171147
len++;
172148
}
173-
if (len > MAXLABEL)
149+
if (len > MAXLABEL) {
150+
free (buf);
174151
return ARES_EBADNAME;
152+
}
175153

176154
/* Encode the length and copy the data. */
177155
*q++ = (unsigned char)len;
@@ -195,14 +173,30 @@ int ares_create_query(const char *name, int dnsclass, int type,
195173
DNS_QUESTION_SET_TYPE(q, type);
196174
DNS_QUESTION_SET_CLASS(q, dnsclass);
197175

176+
q += QFIXEDSZ;
198177
if (max_udp_size)
199178
{
200-
q += QFIXEDSZ;
201179
memset(q, 0, EDNSFIXEDSZ);
202180
q++;
203181
DNS_RR_SET_TYPE(q, T_OPT);
204182
DNS_RR_SET_CLASS(q, max_udp_size);
183+
q += (EDNSFIXEDSZ-1);
184+
}
185+
buflen = (q - buf);
186+
187+
/* Reject names that are longer than the maximum of 255 bytes that's
188+
* specified in RFC 1035 ("To simplify implementations, the total length of
189+
* a domain name (i.e., label octets and label length octets) is restricted
190+
* to 255 octets or less."). */
191+
if (buflen > (MAXCDNAME + HFIXEDSZ + QFIXEDSZ +
192+
(max_udp_size ? EDNSFIXEDSZ : 0))) {
193+
free (buf);
194+
return ARES_EBADNAME;
205195
}
206196

197+
/* we know this fits in an int at this point */
198+
*buflenp = (int) buflen;
199+
*bufp = buf;
200+
207201
return ARES_SUCCESS;
208202
}

0 commit comments

Comments
 (0)