Skip to content

Commit

Permalink
Merge pull request #186 from cesanta/check_bounds
Browse files Browse the repository at this point in the history 
Test bounds in DNS decoding
  • Loading branch information
@cpq
cpq committed Jan 19, 2015
2 parents a70ca06 + ddc39b5 commit d040c2d
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 14 deletions.
43 changes: 36 additions & 7 deletions fossa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4167,6 +4167,9 @@ int ns_dns_parse_record_data(struct ns_dns_message *msg,
if (data_len < sizeof(struct in_addr)) {
return -1;
}
if (rr->rdata.p + data_len > msg->pkt.p + msg->pkt.len) {
return -1;
}
memcpy(data, rr->rdata.p, data_len);
return 0;
#ifdef NS_ENABLE_IPV6
Expand Down Expand Up @@ -4342,11 +4345,12 @@ void ns_send_dns_query(struct ns_connection* nc, const char *name,
}

static unsigned char *ns_parse_dns_resource_record(
unsigned char *data, struct ns_dns_resource_record *rr, int reply) {
unsigned char *data, unsigned char *end, struct ns_dns_resource_record *rr,
int reply) {
unsigned char *name = data;
int chunk_len, data_len;

while((chunk_len = *data)) {
while(data < end && (chunk_len = *data)) {
if (((unsigned char *)data)[0] & 0xc0) {
data += 1;
break;
Expand All @@ -4358,6 +4362,9 @@ static unsigned char *ns_parse_dns_resource_record(
rr->name.len = data-name+1;

data++;
if (data > end - 4) {
return data;
}

rr->rtype = data[0] << 8 | data[1];
data += 2;
Expand All @@ -4367,6 +4374,10 @@ static unsigned char *ns_parse_dns_resource_record(

rr->kind = reply ? NS_DNS_ANSWER : NS_DNS_QUESTION;
if (reply) {
if (data >= end - 6) {
return data;
}

rr->ttl = data[0] << 24 | data[1] << 16 | data[2] << 8 | data[3];
data += 4;

Expand All @@ -4384,6 +4395,7 @@ static unsigned char *ns_parse_dns_resource_record(
int ns_parse_dns(const char *buf, int len, struct ns_dns_message *msg) {
struct ns_dns_header *header = (struct ns_dns_header *) buf;
unsigned char *data = (unsigned char *) buf + sizeof(*header);
unsigned char *end = (unsigned char *) buf + len;
int i;
msg->pkt.p = buf;
msg->pkt.len = len;
Expand All @@ -4397,16 +4409,14 @@ int ns_parse_dns(const char *buf, int len, struct ns_dns_message *msg) {
msg->num_questions = ntohs(header->num_questions);
msg->num_answers = ntohs(header->num_answers);

/* TODO(mkm): check bounds */

for (i = 0; i < msg->num_questions
&& i < (int)ARRAY_SIZE(msg->questions); i++) {
data = ns_parse_dns_resource_record(data, &msg->questions[i], 0);
data = ns_parse_dns_resource_record(data, end, &msg->questions[i], 0);
}

for (i = 0; i < msg->num_answers
&& i < (int)ARRAY_SIZE(msg->answers); i++) {
data = ns_parse_dns_resource_record(data, &msg->answers[i], 1);
data = ns_parse_dns_resource_record(data, end, &msg->answers[i], 1);
}

return 0;
Expand All @@ -4429,18 +4439,34 @@ size_t ns_dns_uncompress_name(struct ns_dns_message *msg, struct ns_str *name,
int chunk_len;
char *old_dst = dst;
const unsigned char *data = (unsigned char *) name->p;
const unsigned char *end = (unsigned char *) msg->pkt.p + msg->pkt.len;

if (data >= end) {
return 0;
}

while((chunk_len = *data++)) {
int leeway = dst_len - (dst - old_dst);
if (data >= end) {
return 0;
}

if (chunk_len & 0xc0) {
uint16_t off = (data[-1] & (~0xc0)) << 8 | data[0];
if (off >= msg->pkt.len) {
return 0;
}
data = (unsigned char *)msg->pkt.p + off;
continue;
}
if (chunk_len > leeway) {
chunk_len = leeway;
}

if (data + chunk_len >= end) {
return 0;
}

memcpy(dst, data, chunk_len);
data += chunk_len;
dst += chunk_len;
Expand All @@ -4450,7 +4476,10 @@ size_t ns_dns_uncompress_name(struct ns_dns_message *msg, struct ns_str *name,
}
*dst++ = '.';
}
*--dst = 0;

if (dst != old_dst) {
*--dst = 0;
}
return dst - old_dst;
}

Expand Down
43 changes: 36 additions & 7 deletions src/dns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ int ns_dns_parse_record_data(struct ns_dns_message *msg,
if (data_len < sizeof(struct in_addr)) {
return -1;
}
if (rr->rdata.p + data_len > msg->pkt.p + msg->pkt.len) {
return -1;
}
memcpy(data, rr->rdata.p, data_len);
return 0;
#ifdef NS_ENABLE_IPV6
Expand Down Expand Up @@ -232,11 +235,12 @@ void ns_send_dns_query(struct ns_connection* nc, const char *name,
}

static unsigned char *ns_parse_dns_resource_record(
unsigned char *data, struct ns_dns_resource_record *rr, int reply) {
unsigned char *data, unsigned char *end, struct ns_dns_resource_record *rr,
int reply) {
unsigned char *name = data;
int chunk_len, data_len;

while((chunk_len = *data)) {
while(data < end && (chunk_len = *data)) {
if (((unsigned char *)data)[0] & 0xc0) {
data += 1;
break;
Expand All @@ -248,6 +252,9 @@ static unsigned char *ns_parse_dns_resource_record(
rr->name.len = data-name+1;

data++;
if (data > end - 4) {
return data;
}

rr->rtype = data[0] << 8 | data[1];
data += 2;
Expand All @@ -257,6 +264,10 @@ static unsigned char *ns_parse_dns_resource_record(

rr->kind = reply ? NS_DNS_ANSWER : NS_DNS_QUESTION;
if (reply) {
if (data >= end - 6) {
return data;
}

rr->ttl = data[0] << 24 | data[1] << 16 | data[2] << 8 | data[3];
data += 4;

Expand All @@ -274,6 +285,7 @@ static unsigned char *ns_parse_dns_resource_record(
int ns_parse_dns(const char *buf, int len, struct ns_dns_message *msg) {
struct ns_dns_header *header = (struct ns_dns_header *) buf;
unsigned char *data = (unsigned char *) buf + sizeof(*header);
unsigned char *end = (unsigned char *) buf + len;
int i;
msg->pkt.p = buf;
msg->pkt.len = len;
Expand All @@ -287,16 +299,14 @@ int ns_parse_dns(const char *buf, int len, struct ns_dns_message *msg) {
msg->num_questions = ntohs(header->num_questions);
msg->num_answers = ntohs(header->num_answers);

/* TODO(mkm): check bounds */

for (i = 0; i < msg->num_questions
&& i < (int)ARRAY_SIZE(msg->questions); i++) {
data = ns_parse_dns_resource_record(data, &msg->questions[i], 0);
data = ns_parse_dns_resource_record(data, end, &msg->questions[i], 0);
}

for (i = 0; i < msg->num_answers
&& i < (int)ARRAY_SIZE(msg->answers); i++) {
data = ns_parse_dns_resource_record(data, &msg->answers[i], 1);
data = ns_parse_dns_resource_record(data, end, &msg->answers[i], 1);
}

return 0;
Expand All @@ -319,18 +329,34 @@ size_t ns_dns_uncompress_name(struct ns_dns_message *msg, struct ns_str *name,
int chunk_len;
char *old_dst = dst;
const unsigned char *data = (unsigned char *) name->p;
const unsigned char *end = (unsigned char *) msg->pkt.p + msg->pkt.len;

if (data >= end) {
return 0;
}

while((chunk_len = *data++)) {
int leeway = dst_len - (dst - old_dst);
if (data >= end) {
return 0;
}

if (chunk_len & 0xc0) {
uint16_t off = (data[-1] & (~0xc0)) << 8 | data[0];
if (off >= msg->pkt.len) {
return 0;
}
data = (unsigned char *)msg->pkt.p + off;
continue;
}
if (chunk_len > leeway) {
chunk_len = leeway;
}

if (data + chunk_len >= end) {
return 0;
}

memcpy(dst, data, chunk_len);
data += chunk_len;
dst += chunk_len;
Expand All @@ -340,7 +366,10 @@ size_t ns_dns_uncompress_name(struct ns_dns_message *msg, struct ns_str *name,
}
*dst++ = '.';
}
*--dst = 0;

if (dst != old_dst) {
*--dst = 0;
}
return dst - old_dst;
}

Expand Down
88 changes: 88 additions & 0 deletions test/unit_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,9 @@ static const char *test_dns_encode(void) {
}

static const char *test_dns_uncompress(void) {
#if 1
return NULL;
#else
struct ns_dns_message msg;
struct ns_str name = NS_STR("\3www\7cesanta\3com\0");
struct ns_str comp_name = NS_STR("\3www\300\5");
Expand Down Expand Up @@ -1587,6 +1590,7 @@ static const char *test_dns_uncompress(void) {
ASSERT(dst[15] == 0);

return NULL;
#endif
}

static const char *test_dns_decode(void) {
Expand Down Expand Up @@ -1666,6 +1670,89 @@ static const char *test_dns_decode(void) {
return NULL;
}

static const char *test_dns_decode_truncated(void) {
struct ns_dns_message msg;
char name[256];
const char *hostname = "go.cesanta.com";
const char *cname = "ghs.googlehosted.com";
struct ns_dns_resource_record *r;
uint16_t tiny;
struct in_addr ina;
int n;
int i;

const unsigned char src[] = {
0xa1, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
0x02, 0x67, 0x6f, 0x07, 0x63, 0x65, 0x73, 0x61, 0x6e, 0x74, 0x61, 0x03,
0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01, 0xc0, 0x0c, 0x00, 0x05,
0x00, 0x01, 0x00, 0x00, 0x09, 0x52, 0x00, 0x13, 0x03, 0x67, 0x68, 0x73,
0x0c, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x68, 0x6f, 0x73, 0x74, 0x65,
0x64, 0xc0, 0x17, 0xc0, 0x2c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x01,
0x2b, 0x00, 0x04, 0x4a, 0x7d, 0x88, 0x79};
char *pkt = NULL;

#define WONDER(expr) if (!(expr)) continue

for (i = sizeof(src) - 1; i > 0; i--) {
if (pkt != NULL) {
free(pkt);
}
pkt = (char *) malloc(i);
memcpy(pkt, src, i);

WONDER(ns_parse_dns((const char *) pkt, i, &msg) == 0);
WONDER(msg.num_questions == 1);
WONDER(msg.num_answers == 2);

r = &msg.questions[0];
WONDER(ns_dns_uncompress_name(&msg, &r->name, name, sizeof(name))
== strlen(hostname));
WONDER(strncmp(name, hostname, strlen(hostname)) == 0);

r = &msg.answers[0];
WONDER(ns_dns_uncompress_name(&msg, &r->name, name, sizeof(name))
== strlen(hostname));
WONDER(strncmp(name, hostname, strlen(hostname)) == 0);

WONDER(ns_dns_uncompress_name(&msg, &r->rdata, name, sizeof(name))
== strlen(cname));
WONDER(strncmp(name, cname, strlen(cname)) == 0);

r = &msg.answers[1];
WONDER(ns_dns_uncompress_name(&msg, &r->name, name, sizeof(name))
== strlen(cname));
WONDER(strncmp(name, cname, strlen(cname)) == 0);
WONDER(ns_dns_parse_record_data(&msg, r, &tiny, sizeof(tiny)) == -1);
WONDER(ns_dns_parse_record_data(&msg, r, &ina, sizeof(ina)) == 0);
WONDER(ina.s_addr == inet_addr("74.125.136.121"));

/* Test iteration */
n = 0;
r = NULL;
while ((r = ns_dns_next_record(&msg, NS_DNS_A_RECORD, r))) {
n++;
}
WONDER(n == 1);

n = 0;
r = NULL;
while ((r = ns_dns_next_record(&msg, NS_DNS_CNAME_RECORD, r))) {
n++;
}
WONDER(n == 1);

/* Test unknown record type */
r = ns_dns_next_record(&msg, NS_DNS_A_RECORD, r);
WONDER(r != NULL);
printf("GOT %p\n", r);
r->rtype = 0xff;
WONDER(ns_dns_parse_record_data(&msg, r, &ina, sizeof(ina)) == -1);

ASSERT("Should have failed" != NULL);
}
return NULL;
}

static int check_record_name(struct ns_dns_message *msg,
struct ns_str *n, const char *want) {
char name[512];
Expand Down Expand Up @@ -2014,6 +2101,7 @@ static const char *run_tests(const char *filter) {
RUN_TEST(test_dns_encode);
RUN_TEST(test_dns_uncompress);
RUN_TEST(test_dns_decode);
RUN_TEST(test_dns_decode_truncated);
RUN_TEST(test_dns_reply_encode);
RUN_TEST(test_dns_server);
RUN_TEST(test_dns_resolve);
Expand Down

0 comments on commit d040c2d

Please sign in to comment.