More boundary checks for mg_dns_parse()

This commit is contained in:
cpq committed 2020-12-17 19:14:17 +00:00
1 parent 4519ea568d
commit 29d31ba919
3 files changed
+37 -20

No files matched your search

+14 -8
View File
@@ -165,25 +165,28 @@ void mg_resolve_cancel(struct mg_mgr *mgr, struct mg_connection *c) {
static size_t mg_dns_parse_name(const uint8_t *s, const uint8_t *e, size_t off,
char *to, size_t tolen, int depth) {
size_t i = 0, j = 0;
if (tolen > 0) to[0] = '\0';
if (depth > 5) return 0;
while (&s[off + i + 1] < e && s[off + i] > 0) {
size_t n = s[off + i];
if (n & 0xc0) {
size_t ptr = (((n & 0x3f) << 8) | s[off + i + 1]) - 12; // 12 is hdr len
if (&s[ptr + 1] < e && (s[ptr] & 0xc0) == 0) {
j = mg_dns_parse_name(s, e, ptr, to, tolen, depth + 1);
mg_dns_parse_name(s, e, ptr, to, tolen, depth + 1);
}
i++;
break;
}
if (&s[off + i + n + 1] >= e) break;
if (j > 0 && j < tolen) to[j++] = '.';
if (j + n < tolen) memcpy(&to[j], &s[off + i + 1], n);
if (j + n + 1 >= tolen) return 0; // Error - overflow
if (j > 0) to[j++] = '.';
memcpy(&to[j], &s[off + i + 1], n);
j += n;
i += n + 1;
to[j] = '\0'; // Zero-terminate this chunk
LOG(LL_DEBUG, ("-- %zu/%zu %zu %zu", i, e - s, j, n));
}
if (j < tolen) to[j] = '\0'; // Zero-terminate the name
to[tolen - 1] = '\0'; // Just in case
if (tolen > 0) to[tolen - 1] = '\0'; // Make sure make sure it is nul-term
return i;
}
@@ -195,14 +198,17 @@ int mg_dns_parse(const uint8_t *buf, size_t len, struct mg_dns_message *dm) {
struct mg_dns_header *h = (struct mg_dns_header *) buf;
const uint8_t *s = buf + sizeof(*h), *e = &buf[len];
size_t i, j = 0, n, ok = 0;
if (len < sizeof(*h)) return ok;
if (len < sizeof(*h)) return ok; // Too small, headers dont fit
if (len > 512) return ok; // Too large, we don't expect that
if (mg_ntohs(h->num_questions) > 2) return 0; // Sanity
if (mg_ntohs(h->num_answers) > 5) return 0; // Sanity
for (i = 0; i < mg_ntohs(h->num_questions); i++) {
j += mg_dns_parse_name(s, e, j, dm->name, sizeof(dm->name), 0) + 5;
// LOG(LL_INFO, ("QUE [%s]", name));
LOG(LL_INFO, ("QUE %zu %zu [%s]", i, j, dm->name));
}
for (i = 0; i < mg_ntohs(h->num_answers); i++) {
j += mg_dns_parse_name(s, e, j, dm->name, sizeof(dm->name), 0) + 9;
// LOG(LL_DEBUG, ("NAME %s", name));
LOG(LL_DEBUG, ("NAME %s", dm->name));
if (&s[j] + 2 > e) break;
n = ((int) s[j] << 8) | s[j + 1];
if (&s[j] + 2 + n > e) break;
+14 -8
View File
@@ -39,25 +39,28 @@ void mg_resolve_cancel(struct mg_mgr *mgr, struct mg_connection *c) {
static size_t mg_dns_parse_name(const uint8_t *s, const uint8_t *e, size_t off,
char *to, size_t tolen, int depth) {
size_t i = 0, j = 0;
if (tolen > 0) to[0] = '\0';
if (depth > 5) return 0;
while (&s[off + i + 1] < e && s[off + i] > 0) {
size_t n = s[off + i];
if (n & 0xc0) {
size_t ptr = (((n & 0x3f) << 8) | s[off + i + 1]) - 12; // 12 is hdr len
if (&s[ptr + 1] < e && (s[ptr] & 0xc0) == 0) {
j = mg_dns_parse_name(s, e, ptr, to, tolen, depth + 1);
mg_dns_parse_name(s, e, ptr, to, tolen, depth + 1);
}
i++;
break;
}
if (&s[off + i + n + 1] >= e) break;
if (j > 0 && j < tolen) to[j++] = '.';
if (j + n < tolen) memcpy(&to[j], &s[off + i + 1], n);
if (j + n + 1 >= tolen) return 0; // Error - overflow
if (j > 0) to[j++] = '.';
memcpy(&to[j], &s[off + i + 1], n);
j += n;
i += n + 1;
to[j] = '\0'; // Zero-terminate this chunk
LOG(LL_DEBUG, ("-- %zu/%zu %zu %zu", i, e - s, j, n));
}
if (j < tolen) to[j] = '\0'; // Zero-terminate the name
to[tolen - 1] = '\0'; // Just in case
if (tolen > 0) to[tolen - 1] = '\0'; // Make sure make sure it is nul-term
return i;
}
@@ -69,14 +72,17 @@ int mg_dns_parse(const uint8_t *buf, size_t len, struct mg_dns_message *dm) {
struct mg_dns_header *h = (struct mg_dns_header *) buf;
const uint8_t *s = buf + sizeof(*h), *e = &buf[len];
size_t i, j = 0, n, ok = 0;
if (len < sizeof(*h)) return ok;
if (len < sizeof(*h)) return ok; // Too small, headers dont fit
if (len > 512) return ok; // Too large, we don't expect that
if (mg_ntohs(h->num_questions) > 2) return 0; // Sanity
if (mg_ntohs(h->num_answers) > 5) return 0; // Sanity
for (i = 0; i < mg_ntohs(h->num_questions); i++) {
j += mg_dns_parse_name(s, e, j, dm->name, sizeof(dm->name), 0) + 5;
// LOG(LL_INFO, ("QUE [%s]", name));
LOG(LL_INFO, ("QUE %zu %zu [%s]", i, j, dm->name));
}
for (i = 0; i < mg_ntohs(h->num_answers); i++) {
j += mg_dns_parse_name(s, e, j, dm->name, sizeof(dm->name), 0) + 9;
// LOG(LL_DEBUG, ("NAME %s", name));
LOG(LL_DEBUG, ("NAME %s", dm->name));
if (&s[j] + 2 > e) break;
n = ((int) s[j] << 8) | s[j + 1];
if (&s[j] + 2 + n > e) break;
+9 -4
View File
@@ -964,8 +964,6 @@ static void test_dns(void) {
0x74, 0x61, 0x03, 0x63, 0x6f, 0x6d, 0, 0, 1,
0, 1, 0xc0, 0x0c, 0, 1, 0, 1, 0,
0, 2, 0x57, 0, 4, 0x94, 0xfb, 0x36, 0xec};
// char *data = mg_file_read("dns.bin");
// ASSERT(data != NULL);
ASSERT(mg_dns_parse(NULL, 0, &dm) == 0);
ASSERT(mg_dns_parse(data, sizeof(data), &dm) == 1);
ASSERT(strcmp(dm.name, "cesanta.com") == 0);
@@ -973,8 +971,15 @@ static void test_dns(void) {
memset(&dm, 0, sizeof(dm));
ASSERT(mg_dns_parse(data, sizeof(data), &dm) == 1);
ASSERT(strcmp(dm.name, "") == 0);
// ASSERT(mg_dns_parse((uint8_t *) data, strlen(data), &dm) == 0);
// free(data);
#if 0
{
char *data = mg_file_read("dns.bin");
ASSERT(data != NULL);
ASSERT(mg_dns_parse((uint8_t *) data, mg_file_size("dns.bin"), &dm) == 0);
free(data);
}
#endif
}
static void test_util(void) {