Chiebot-Mirror
/
c-ares
mirror of https://github.com/c-ares/c-ares.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Validate hostnames in DNS responses and discard from malicious servers (#406)

Browse Source 
To prevent possible users having XSS issues due to intentionally malformed DNS replies, validate hostnames returned in responses and return EBADRESP if they are not valid.

It is not clear what legitimate issues this may cause at this point.

Bug Reported By: philipp.jeitner@sit.fraunhofer.de
Fix By: Brad House (@bradh352)
pull/408/head
Brad House 3 years ago committed by GitHub
parent 44c009b8e6
commit c9b6c605b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 78 additions and 28 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 6
      src/lib/ares__parse_into_addrinfo.c
  2. 66
      src/lib/ares_expand_name.c
  3. 6
      src/lib/ares_parse_ns_reply.c
  4. 8
      src/lib/ares_parse_ptr_reply.c
  5. 8
      src/lib/ares_parse_soa_reply.c
  6. 6
      src/lib/ares_private.h
  7. 3
      src/lib/ares_process.c
  8. 3
      test/ares-test-parse.cc

6
src/lib/ares__parse_into_addrinfo.c
Unescape View File

@ -70,7 +70,7 @@ int ares__parse_into_addrinfo2(const unsigned char *abuf,
/* Expand the name from the question, and skip past the question. */
aptr = abuf + HFIXEDSZ;
status = ares__expand_name_for_response(aptr, abuf, alen, question_hostname, &len);
status = ares__expand_name_for_response(aptr, abuf, alen, question_hostname, &len, 0);
if (status != ARES_SUCCESS)
return status;
if (aptr + len + QFIXEDSZ > abuf + alen)
@ -86,7 +86,7 @@ int ares__parse_into_addrinfo2(const unsigned char *abuf,
for (i = 0; i < (int)ancount; i++)
{
/* Decode the RR up to the data field. */
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_name, &len);
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_name, &len, 0);
if (status != ARES_SUCCESS)
{
rr_name = NULL;
@ -188,7 +188,7 @@ int ares__parse_into_addrinfo2(const unsigned char *abuf,
{
got_cname = 1;
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_data,
&len);
&len, 1);
if (status != ARES_SUCCESS)
{
goto failed_stat;

66
src/lib/ares_expand_name.c
Unescape View File

@ -30,7 +30,7 @@
#define MAX_INDIRS 50
static int name_length(const unsigned char *encoded, const unsigned char *abuf,
int alen);
int alen, int is_hostname);
/* Reserved characters for names that need to be escaped */
static int is_reservedch(int ch)
@ -52,6 +52,31 @@ static int is_reservedch(int ch)
return 0;
}
static int ares__isprint(int ch)
{
if (ch >= 0x20 && ch <= 0x7E)
return 1;
return 0;
}
/* Character set allowed by hostnames */
static int is_hostnamech(int ch)
{
/* [A-Za-z0-9-.]
* Don't use isalnum() as it is locale-specific
*/
if (ch >= 'A' && ch <= 'Z')
return 1;
if (ch >= 'a' && ch <= 'z')
return 1;
if (ch >= '0' && ch <= '9')
return 1;
if (ch == '-' || ch == '.')
return 1;
return 0;
}
/* Expand an RFC1035-encoded domain name given by encoded. The
* containing message is given by abuf and alen. The result given by
* *s, which is set to a NUL-terminated allocated buffer. *enclen is
@ -74,10 +99,15 @@ static int is_reservedch(int ch)
*
* Since the expanded name uses '.' as a label separator, we use
* backslashes to escape periods or backslashes in the expanded name.
*
* If the result is expected to be a hostname, then no escaped data is allowed
* and will return error.
*/
int ares_expand_name(const unsigned char *encoded, const unsigned char *abuf,
int alen, char **s, long *enclen)
int ares__expand_name_validated(const unsigned char *encoded,
const unsigned char *abuf,
int alen, char **s, long *enclen,
int is_hostname)
{
int len, indir = 0;
char *q;
@ -87,7 +117,7 @@ int ares_expand_name(const unsigned char *encoded, const unsigned char *abuf,
size_t uns;
} nlen;
nlen.sig = name_length(encoded, abuf, alen);
nlen.sig = name_length(encoded, abuf, alen, is_hostname);
if (nlen.sig < 0)
return ARES_EBADNAME;
@ -135,9 +165,8 @@ int ares_expand_name(const unsigned char *encoded, const unsigned char *abuf,
{
/* Output as \DDD for consistency with RFC1035 5.1, except
* for the special case of a root name response */
if (!isprint(*p) && !(name_len == 1 && *p == 0))
if (!ares__isprint(*p) && !(name_len == 1 && *p == 0))
{
*q++ = '\\';
*q++ = '0' + *p / 100;
*q++ = '0' + (*p % 100) / 10;
@ -170,11 +199,18 @@ int ares_expand_name(const unsigned char *encoded, const unsigned char *abuf,
return ARES_SUCCESS;
}
int ares_expand_name(const unsigned char *encoded, const unsigned char *abuf,
int alen, char **s, long *enclen)
{
return ares__expand_name_validated(encoded, abuf, alen, s, enclen, 0);
}
/* Return the length of the expansion of an encoded domain name, or
* -1 if the encoding is invalid.
*/
static int name_length(const unsigned char *encoded, const unsigned char *abuf,
int alen)
int alen, int is_hostname)
{
int n = 0, offset, indir = 0, top;
@ -212,16 +248,22 @@ static int name_length(const unsigned char *encoded, const unsigned char *abuf,
while (offset--)
{
if (!isprint(*encoded) && !(name_len == 1 && *encoded == 0))
if (!ares__isprint(*encoded) && !(name_len == 1 && *encoded == 0))
{
if (is_hostname)
return -1;
n += 4;
}
else if (is_reservedch(*encoded))
{
if (is_hostname)
return -1;
n += 2;
}
else
{
if (is_hostname && !is_hostnamech(*encoded))
return -1;
n += 1;
}
encoded++;
@ -244,12 +286,14 @@ static int name_length(const unsigned char *encoded, const unsigned char *abuf,
return (n) ? n - 1 : n;
}
/* Like ares_expand_name but returns EBADRESP in case of invalid input. */
/* Like ares_expand_name_validated but returns EBADRESP in case of invalid
* input. */
int ares__expand_name_for_response(const unsigned char *encoded,
const unsigned char *abuf, int alen,
char **s, long *enclen)
char **s, long *enclen, int is_hostname)
{
int status = ares_expand_name(encoded, abuf, alen, s, enclen);
int status = ares__expand_name_validated(encoded, abuf, alen, s, enclen,
is_hostname);
if (status == ARES_EBADNAME)
status = ARES_EBADRESP;
return status;

6
src/lib/ares_parse_ns_reply.c
Unescape View File

@ -62,7 +62,7 @@ int ares_parse_ns_reply( const unsigned char* abuf, int alen,
/* Expand the name from the question, and skip past the question. */
aptr = abuf + HFIXEDSZ;
status = ares__expand_name_for_response( aptr, abuf, alen, &hostname, &len);
status = ares__expand_name_for_response( aptr, abuf, alen, &hostname, &len, 0);
if ( status != ARES_SUCCESS )
return status;
if ( aptr + len + QFIXEDSZ > abuf + alen )
@ -85,7 +85,7 @@ int ares_parse_ns_reply( const unsigned char* abuf, int alen,
for ( i = 0; i < ( int ) ancount; i++ )
{
/* Decode the RR up to the data field. */
status = ares__expand_name_for_response( aptr, abuf, alen, &rr_name, &len );
status = ares__expand_name_for_response( aptr, abuf, alen, &rr_name, &len, 0);
if ( status != ARES_SUCCESS )
break;
aptr += len;
@ -110,7 +110,7 @@ int ares_parse_ns_reply( const unsigned char* abuf, int alen,
{
/* Decode the RR data and add it to the nameservers list */
status = ares__expand_name_for_response( aptr, abuf, alen, &rr_data,
&len);
&len, 1);
if ( status != ARES_SUCCESS )
{
ares_free(rr_name);

8
src/lib/ares_parse_ptr_reply.c
Unescape View File

@ -63,7 +63,7 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
/* Expand the name from the question, and skip past the question. */
aptr = abuf + HFIXEDSZ;
status = ares__expand_name_for_response(aptr, abuf, alen, &ptrname, &len);
status = ares__expand_name_for_response(aptr, abuf, alen, &ptrname, &len, 0);
if (status != ARES_SUCCESS)
return status;
if (aptr + len + QFIXEDSZ > abuf + alen)
@ -84,7 +84,7 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
for (i = 0; i < (int)ancount; i++)
{
/* Decode the RR up to the data field. */
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_name, &len);
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_name, &len, 0);
if (status != ARES_SUCCESS)
break;
aptr += len;
@ -110,7 +110,7 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
{
/* Decode the RR data and set hostname to it. */
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_data,
&len);
&len, 1);
if (status != ARES_SUCCESS)
{
ares_free(rr_name);
@ -146,7 +146,7 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
{
/* Decode the RR data and replace ptrname with it. */
status = ares__expand_name_for_response(aptr, abuf, alen, &rr_data,
&len);
&len, 1);
if (status != ARES_SUCCESS)
{
ares_free(rr_name);

8
src/lib/ares_parse_soa_reply.c
Unescape View File

@ -60,7 +60,7 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
aptr = abuf + HFIXEDSZ;
/* query name */
status = ares__expand_name_for_response(aptr, abuf, alen, &qname, &len);
status = ares__expand_name_for_response(aptr, abuf, alen, &qname, &len, 0);
if (status != ARES_SUCCESS)
goto failed_stat;
@ -83,7 +83,7 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
for (i = 0; i < ancount; i++)
{
rr_name = NULL;
status = ares__expand_name_for_response (aptr, abuf, alen, &rr_name, &len);
status = ares__expand_name_for_response (aptr, abuf, alen, &rr_name, &len, 0);
if (status != ARES_SUCCESS)
{
ares_free(rr_name);
@ -120,7 +120,7 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
/* nsname */
status = ares__expand_name_for_response(aptr, abuf, alen, &soa->nsname,
&len);
&len, 0);
if (status != ARES_SUCCESS)
{
ares_free(rr_name);
@ -130,7 +130,7 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
/* hostmaster */
status = ares__expand_name_for_response(aptr, abuf, alen,
&soa->hostmaster, &len);
&soa->hostmaster, &len, 0);
if (status != ARES_SUCCESS)
{
ares_free(rr_name);

6
src/lib/ares_private.h
Unescape View File

@ -356,9 +356,13 @@ int ares__read_line(FILE *fp, char **buf, size_t *bufsize);
void ares__free_query(struct query *query);
unsigned short ares__generate_new_id(rc4_key* key);
struct timeval ares__tvnow(void);
int ares__expand_name_validated(const unsigned char *encoded,
const unsigned char *abuf,
int alen, char **s, long *enclen,
int is_hostname);
int ares__expand_name_for_response(const unsigned char *encoded,
const unsigned char *abuf, int alen,
char **s, long *enclen);
char **s, long *enclen, int is_hostname);
void ares__init_servers_state(ares_channel channel);
void ares__destroy_servers_state(ares_channel channel);
int ares__parse_qtype_reply(const unsigned char* abuf, int alen, int* qtype);

3
src/lib/ares_process.c
Unescape View File

@ -605,8 +605,7 @@ static void process_answer(ares_channel channel, unsigned char *abuf,
packetsz = PACKETSZ;
/* If we use EDNS and server answers with FORMERR without an OPT RR, the protocol
* extension is not understood by the responder. We must retry the query
* without EDNS enabled.
*/
* without EDNS enabled. */
if (channel->flags & ARES_FLAG_EDNS)
{
packetsz = channel->ednspsz;

3
test/ares-test-parse.cc
Unescape View File

@ -60,6 +60,8 @@ TEST_F(LibraryTest, ParseIndirectRootName) {
ares_free_hostent(host);
}
#if 0 /* We are validating hostnames now, its not clear how this would ever be valid */
TEST_F(LibraryTest, ParseEscapedName) {
std::vector<byte> data = {
0x12, 0x34, // qid
@ -105,6 +107,7 @@ TEST_F(LibraryTest, ParseEscapedName) {
EXPECT_EQ('c', hent.name_[6]);
ares_free_hostent(host);
}
#endif
TEST_F(LibraryTest, ParsePartialCompressedName) {
std::vector<byte> data = {

Write Preview
Loading…
Cancel
Save