From 127a4a90b0d03ebf55ad44d25f75b30c3a6fb728 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 7 May 2013 13:54:58 -0700 Subject: [PATCH] [master] more map file sanity checks (not adding a new CHANGES note because this is an extension of the previous one, change #3570.) --- lib/dns/include/dns/name.h | 5 ++ lib/dns/name.c | 40 ++++++++++ lib/dns/rbt.c | 50 +++++++++--- lib/dns/rbtdb.c | 25 +++++- lib/dns/tests/rbt_test.c | 151 ++++++++++++++++++++++++++++++------- lib/dns/win32/libdns.def | 1 + 6 files changed, 231 insertions(+), 41 deletions(-) diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h index 1a88e53264..bf225854d3 100644 --- a/lib/dns/include/dns/name.h +++ b/lib/dns/include/dns/name.h @@ -231,6 +231,11 @@ dns_name_invalidate(dns_name_t *name); * \li If the name had a dedicated buffer, that association is ended. */ +isc_boolean_t +dns_name_isvalid(const dns_name_t *name); +/*%< + * Check whether 'name' points to a valid dns_name + */ /*** *** Dedicated Buffers diff --git a/lib/dns/name.c b/lib/dns/name.c index 7fb21e138c..9b6bfeb1cd 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -239,6 +239,46 @@ dns_name_invalidate(dns_name_t *name) { ISC_LINK_INIT(name, link); } +isc_boolean_t +dns_name_isvalid(const dns_name_t *name) { + unsigned char *ndata, *offsets; + unsigned int offset, count, length, nlabels; + + if (!VALID_NAME(name)) + return (ISC_FALSE); + + if (name->length > 255U || name->labels > 127U) + return (ISC_FALSE); + + ndata = name->ndata; + length = name->length; + offsets = name->offsets; + offset = 0; + nlabels = 0; + + while (offset != length) { + count = *ndata; + if (count > 63U) + return (ISC_FALSE); + if (offsets != NULL && offsets[nlabels] != offset) + return (ISC_FALSE); + + nlabels++; + offset += count + 1; + ndata += count + 1; + if (offset > length) + return (ISC_FALSE); + + if (count == 0) + break; + } + + if (nlabels != name->labels || offset != name->length) + return (ISC_FALSE); + + return (ISC_TRUE); +} + void dns_name_setbuffer(dns_name_t *name, isc_buffer_t *buffer) { /* diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 007f722b94..5692bca78a 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -273,8 +273,7 @@ getdata(dns_rbtnode_t *node, file_header_t *header) { */ static inline void -NODENAME(dns_rbtnode_t *node, dns_name_t *name) -{ +NODENAME(dns_rbtnode_t *node, dns_name_t *name) { name->length = NAMELEN(node); name->labels = OFFSETLEN(node); name->ndata = NAME(node); @@ -348,9 +347,12 @@ static inline void hash_node(dns_rbt_t *rbt, dns_rbtnode_t *node, dns_name_t *name); static inline void unhash_node(dns_rbt_t *rbt, dns_rbtnode_t *node); +static void +rehash(dns_rbt_t *rbt, unsigned int newcount); #else #define hash_node(rbt, node, name) #define unhash_node(rbt, node) +#define rehash(rbt, newcount) #endif static inline void @@ -706,10 +708,16 @@ treefix(dns_rbt_t *rbt, void *base, size_t filesize, dns_rbtnode_t *n, if (n == NULL) return (ISC_R_SUCCESS); + CONFIRM((void *) n >= base); + CONFIRM((char *) n - (char *) base <= (int) nodemax); + CONFIRM(DNS_RBTNODE_VALID(n)); + dns_name_init(&nodename, NULL); NODENAME(n, &nodename); fullname = &nodename; + CONFIRM(dns_name_isvalid(fullname)); + if (!dns_name_isabsolute(&nodename)) { dns_fixedname_init(&fixed); fullname = dns_fixedname_name(&fixed); @@ -739,6 +747,7 @@ treefix(dns_rbt_t *rbt, void *base, size_t filesize, dns_rbtnode_t *n, CONFIRM(n->down <= (dns_rbtnode_t *) nodemax); n->down = getdown(n, rbt->mmap_location); n->down_is_relative = 0; + CONFIRM(n->down > (dns_rbtnode_t *) n); CONFIRM(DNS_RBTNODE_VALID(n->down)); } else CONFIRM(n->down == NULL); @@ -747,6 +756,7 @@ treefix(dns_rbt_t *rbt, void *base, size_t filesize, dns_rbtnode_t *n, CONFIRM(n->parent <= (dns_rbtnode_t *) nodemax); n->parent = getparent(n, rbt->mmap_location); n->parent_is_relative = 0; + CONFIRM(n->parent < (dns_rbtnode_t *) n); CONFIRM(DNS_RBTNODE_VALID(n->parent)); } else CONFIRM(n->parent == NULL); @@ -755,6 +765,7 @@ treefix(dns_rbt_t *rbt, void *base, size_t filesize, dns_rbtnode_t *n, CONFIRM(n->data <= (void *) filesize); n->data = getdata(n, rbt->mmap_location); n->data_is_relative = 0; + CONFIRM(n->data > (void *) n); } else CONFIRM(n->data == NULL); @@ -774,6 +785,7 @@ treefix(dns_rbt_t *rbt, void *base, size_t filesize, dns_rbtnode_t *n, if (datafixer != NULL && n->data != NULL) CHECK(datafixer(n, base, filesize, sha1)); + rbt->nodecount++; node_data = (unsigned char *) n + sizeof(dns_rbtnode_t); datasize = NODE_SIZE(n) - sizeof(dns_rbtnode_t); @@ -809,6 +821,7 @@ dns_rbt_deserialize_tree(void *base_address, size_t filesize, isc_sha1_t sha1; REQUIRE(originp == NULL || *originp == NULL); + REQUIRE(rbtp != NULL && *rbtp == NULL); isc_sha1_init(&sha1); @@ -820,26 +833,35 @@ dns_rbt_deserialize_tree(void *base_address, size_t filesize, #ifdef DNS_RDATASET_FIXED if (header->rdataset_fixed != 1) { - return (ISC_R_INVALIDFILE); + result = ISC_R_INVALIDFILE; + goto cleanup; } #else if (header->rdataset_fixed != 0) { - return (ISC_R_INVALIDFILE); + result = ISC_R_INVALIDFILE; + goto cleanup; } #endif if (header->ptrsize != (isc_uint32_t) sizeof(void *)) { - return (ISC_R_INVALIDFILE); + result = ISC_R_INVALIDFILE; + goto cleanup; } if (header->bigendian != (1 == htonl(1)) ? 1 : 0) { - return (ISC_R_INVALIDFILE); + result = ISC_R_INVALIDFILE; + goto cleanup; } /* Copy other data items from the header into our rbt. */ rbt->root = (dns_rbtnode_t *)((char *)base_address + header_offset + header->first_node_offset); - rbt->nodecount = header->nodecount; + + if ((header->nodecount * sizeof(dns_rbtnode_t)) > filesize) { + result = ISC_R_INVALIDFILE; + goto cleanup; + } + rehash(rbt, header->nodecount); CHECK(treefix(rbt, base_address, filesize, rbt->root, dns_rootname, datafixer, &sha1)); @@ -859,6 +881,9 @@ dns_rbt_deserialize_tree(void *base_address, size_t filesize, if (originp != NULL) *originp = rbt->root; + if (header->nodecount != rbt->nodecount) + result = ISC_R_INVALIDFILE; + cleanup: if (result != ISC_R_SUCCESS) { rbt->root = NULL; @@ -2186,7 +2211,7 @@ inithash(dns_rbt_t *rbt) { } static void -rehash(dns_rbt_t *rbt) { +rehash(dns_rbt_t *rbt, unsigned int newcount) { unsigned int oldsize; dns_rbtnode_t **oldtable; dns_rbtnode_t *node; @@ -2195,7 +2220,9 @@ rehash(dns_rbt_t *rbt) { oldsize = rbt->hashsize; oldtable = rbt->hashtable; - rbt->hashsize = rbt->hashsize * 2 + 1; + do { + rbt->hashsize = rbt->hashsize * 2 + 1; + } while (newcount >= (rbt->hashsize * 3)); rbt->hashtable = isc_mem_get(rbt->mctx, rbt->hashsize * sizeof(dns_rbtnode_t *)); if (rbt->hashtable == NULL) { @@ -2225,11 +2252,10 @@ rehash(dns_rbt_t *rbt) { static inline void hash_node(dns_rbt_t *rbt, dns_rbtnode_t *node, dns_name_t *name) { - REQUIRE(DNS_RBTNODE_VALID(node)); - if (rbt->nodecount >= (rbt->hashsize *3)) - rehash(rbt); + if (rbt->nodecount >= (rbt->hashsize * 3)) + rehash(rbt, rbt->nodecount); hash_add_node(rbt, node, name); } diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 35951cf3d6..52cd8e2880 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -7021,7 +7021,11 @@ rbt_datafixer(dns_rbtnode_t *rbtnode, void *base, size_t filesize, header->node = rbtnode; header->node_is_relative = 0; if (header->next != NULL) { - header->next = (rdatasetheader_t *)(p + size); + size_t cooked = dns_rbt_serialize_align(size); + if ((uintptr_t)header->next != + (p - (unsigned char *)base) + cooked) + return (ISC_R_INVALIDFILE); + header->next = (rdatasetheader_t *)(p + cooked); header->next_is_relative = 0; if ((header->next < (rdatasetheader_t *) base) || (header->next > (rdatasetheader_t *) limit)) @@ -7223,9 +7227,10 @@ rbt_datawriter(FILE *rbtfile, unsigned char *data, isc_uint32_t serial, isc_sha1_t *sha1) { rdatasetheader_t newheader; rdatasetheader_t *header = (rdatasetheader_t *) data, *next; - size_t where, size; + size_t where, size, cooked; unsigned char *p; isc_result_t result; + static const char pad[sizeof(char *)]; REQUIRE(rbtfile != NULL); REQUIRE(data != NULL); @@ -7256,8 +7261,13 @@ rbt_datawriter(FILE *rbtfile, unsigned char *data, isc_uint32_t serial, newheader.node_is_relative = 1; newheader.serial = 1; + /* + * Round size up to the next pointer sized offset so it + * will be properly aligned when read back in. + */ + cooked = dns_rbt_serialize_align(size); if (next != NULL) { - newheader.next = (rdatasetheader_t *) (where + size); + newheader.next = (rdatasetheader_t *) (where + cooked); newheader.next_is_relative = 1; } @@ -7281,6 +7291,15 @@ rbt_datawriter(FILE *rbtfile, unsigned char *data, isc_uint32_t serial, size - sizeof(rdatasetheader_t), 1, rbtfile, NULL); if (result != ISC_R_SUCCESS) return (result); + /* + * Pad to force alignment. + */ + if (cooked != size) { + result = isc_stdio_write(pad, cooked - size, 1, + rbtfile, NULL); + if (result != ISC_R_SUCCESS) + return (result); + } } return (ISC_R_SUCCESS); diff --git a/lib/dns/tests/rbt_test.c b/lib/dns/tests/rbt_test.c index 577ed49ee7..f3099ca3d3 100644 --- a/lib/dns/tests/rbt_test.c +++ b/lib/dns/tests/rbt_test.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -55,6 +56,10 @@ #include +#ifndef MAP_FILE +#define MAP_FILE 0 +#endif + typedef struct data_holder { int len; const char *data; @@ -109,8 +114,8 @@ write_data(FILE *file, unsigned char *datap, isc_uint32_t serial, uintptr_t where = ftell(file); UNUSED(serial); - UNUSED(sha1); + REQUIRE(sha1 != NULL); REQUIRE(data != NULL); REQUIRE((data->len == 0 && data->data == NULL) || (data->len != 0 && data->data != NULL)); @@ -120,10 +125,12 @@ write_data(FILE *file, unsigned char *datap, isc_uint32_t serial, ? NULL : (char *)(where + sizeof(data_holder_t))); + isc_sha1_update(sha1, (void *)&temp, sizeof(temp)); ret = fwrite(&temp, sizeof(data_holder_t), 1, file); if (ret != 1) return (ISC_R_FAILURE); if (data->len > 0) { + isc_sha1_update(sha1, (const void *)data->data, data->len); ret = fwrite(data->data, data->len, 1, file); if (ret != 1) return (ISC_R_FAILURE); @@ -135,22 +142,37 @@ write_data(FILE *file, unsigned char *datap, isc_uint32_t serial, static isc_result_t fix_data(dns_rbtnode_t *p, void *base, size_t max, isc_sha1_t *sha1) { data_holder_t *data = p->data; + size_t size; UNUSED(base); UNUSED(max); - REQUIRE(data != NULL); - REQUIRE((data->len == 0 && data->data == NULL) || - (data->len != 0 && data->data != NULL)); - - UNUSED(sha1); + REQUIRE(sha1 != NULL); + REQUIRE(p != NULL); printf("fixing data: len %d, data %p\n", data->len, data->data); + if (data == NULL || + (data->len == 0 && data->data != NULL) || + (data->len != 0 && data->data == NULL)) + return (ISC_R_INVALIDFILE); + + size = max - ((char *)p - (char *)base); + + if (data->len > (int) size || data->data > (const char *) max) { + printf("data invalid\n"); + return (ISC_R_INVALIDFILE); + } + + isc_sha1_update(sha1, (void *)data, sizeof(*data)); + data->data = (data->len == 0) ? NULL : (char *)data + sizeof(data_holder_t); + if (data->len > 0) + isc_sha1_update(sha1, (const void *)data->data, data->len); + return (ISC_R_SUCCESS); } @@ -158,8 +180,7 @@ fix_data(dns_rbtnode_t *p, void *base, size_t max, isc_sha1_t *sha1) { * Load test data into the RBT. */ static void -add_test_data(isc_mem_t *mctx, dns_rbt_t *rbt) -{ +add_test_data(isc_mem_t *mctx, dns_rbt_t *rbt) { char buffer[1024]; isc_buffer_t b; isc_result_t result; @@ -197,8 +218,7 @@ add_test_data(isc_mem_t *mctx, dns_rbt_t *rbt) * Walk the tree and ensure that all the test nodes are present. */ static void -check_test_data(dns_rbt_t *rbt) -{ +check_test_data(dns_rbt_t *rbt) { char buffer[1024]; char *arg; dns_fixedname_t fname; @@ -244,11 +264,11 @@ data_printer(FILE *out, void *datap) fprintf(out, "%d bytes, %s", data->len, data->data); } -ATF_TC(isc_rbt); -ATF_TC_HEAD(isc_rbt, tc) { +ATF_TC(rbt); +ATF_TC_HEAD(rbt, tc) { atf_tc_set_md_var(tc, "descr", "Test the creation of an rbt"); } -ATF_TC_BODY(isc_rbt, tc) { +ATF_TC_BODY(rbt, tc) { dns_rbt_t *rbt = NULL; isc_result_t result; @@ -272,11 +292,11 @@ ATF_TC_BODY(isc_rbt, tc) { dns_test_end(); } -ATF_TC(isc_serialize_rbt); -ATF_TC_HEAD(isc_serialize_rbt, tc) { +ATF_TC(serialize); +ATF_TC_HEAD(serialize, tc) { atf_tc_set_md_var(tc, "descr", "Test writing an rbt to file"); } -ATF_TC_BODY(isc_serialize_rbt, tc) { +ATF_TC_BODY(serialize, tc) { dns_rbt_t *rbt = NULL; isc_result_t result; FILE *rbtfile = NULL; @@ -314,9 +334,6 @@ ATF_TC_BODY(isc_serialize_rbt, tc) { */ printf("deserialization begins.\n"); -#ifndef MAP_FILE -#define MAP_FILE 0 -#endif /* * Map in the whole file in one go */ @@ -326,6 +343,7 @@ ATF_TC_BODY(isc_serialize_rbt, tc) { PROT_READ|PROT_WRITE, MAP_FILE|MAP_PRIVATE, fd, 0); ATF_REQUIRE(base != NULL && base != MAP_FAILED); + close(fd); result = dns_rbt_deserialize_tree(base, filesize, 0, mctx, delete_data, NULL, @@ -346,11 +364,91 @@ ATF_TC_BODY(isc_serialize_rbt, tc) { dns_test_end(); } -ATF_TC(dns_rbt_serialize_align); -ATF_TC_HEAD(dns_rbt_serialize_align, tc) { - atf_tc_set_md_var(tc, "descr", "Test the dns_rbt_serialize_align() function."); +ATF_TC(deserialize_corrupt); +ATF_TC_HEAD(deserialize_corrupt, tc) { + atf_tc_set_md_var(tc, "descr", "Test reading a corrupt map file"); } -ATF_TC_BODY(dns_rbt_serialize_align, tc) { +ATF_TC_BODY(deserialize_corrupt, tc) { + dns_rbt_t *rbt = NULL; + isc_result_t result; + FILE *rbtfile = NULL; + long offset; + int fd; + off_t filesize = 0; + char *base, *p, *q; + isc_uint32_t r; + int i; + + UNUSED(tc); + + isc_mem_debugging = ISC_MEM_DEBUGRECORD; + + result = dns_test_begin(NULL, ISC_TRUE); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* Set up map file */ + result = dns_rbt_create(mctx, delete_data, NULL, &rbt); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + + add_test_data(mctx, rbt); + rbtfile = fopen("./zone.bin", "w+b"); + ATF_REQUIRE(rbtfile != NULL); + result = dns_rbt_serialize_tree(rbtfile, rbt, write_data, 0, &offset); + ATF_REQUIRE(result == ISC_R_SUCCESS); + dns_rbt_destroy(&rbt); + + /* Read back with random fuzzing */ + for (i = 0; i < 256; i++) { + dns_rbt_t *rbt_deserialized = NULL; + + fd = open("zone.bin", O_RDWR); + isc_file_getsizefd(fd, &filesize); + base = mmap(NULL, filesize, + PROT_READ|PROT_WRITE, + MAP_FILE|MAP_PRIVATE, fd, 0); + ATF_REQUIRE(base != NULL && base != MAP_FAILED); + close(fd); + + /* Randomly fuzz a portion of the memory */ + isc_random_get(&r); + p = base + (r % filesize); + q = base + filesize; + isc_random_get(&r); + q -= (r % (q - p)); + while (p++ < q) { + isc_random_get(&r); + *p = r & 0xff; + } + + result = dns_rbt_deserialize_tree(base, filesize, 0, mctx, + delete_data, NULL, + fix_data, NULL, + &rbt_deserialized); + printf("%d: %s\n", i, isc_result_totext(result)); + + /* Test to make sure we have a valid tree */ + ATF_REQUIRE(result == ISC_R_SUCCESS || + result == ISC_R_INVALIDFILE); + if (result != ISC_R_SUCCESS) + ATF_REQUIRE(rbt_deserialized == NULL); + + if (rbt_deserialized != NULL) + dns_rbt_destroy(&rbt_deserialized); + + munmap(base, filesize); + } + + unlink("zone.bin"); + dns_test_end(); +} + + +ATF_TC(serialize_align); +ATF_TC_HEAD(serialize_align, tc) { + atf_tc_set_md_var(tc, "descr", + "Test the dns_rbt_serialize_align() function."); +} +ATF_TC_BODY(serialize_align, tc) { UNUSED(tc); ATF_CHECK(dns_rbt_serialize_align(0) == 0); @@ -371,9 +469,10 @@ ATF_TC_BODY(dns_rbt_serialize_align, tc) { * Main */ ATF_TP_ADD_TCS(tp) { - ATF_TP_ADD_TC(tp, isc_rbt); - ATF_TP_ADD_TC(tp, isc_serialize_rbt); - ATF_TP_ADD_TC(tp, dns_rbt_serialize_align); + ATF_TP_ADD_TC(tp, rbt); + ATF_TP_ADD_TC(tp, serialize); + ATF_TP_ADD_TC(tp, deserialize_corrupt); + ATF_TP_ADD_TC(tp, serialize_align); return (atf_no_error()); } diff --git a/lib/dns/win32/libdns.def b/lib/dns/win32/libdns.def index fddc2bd303..269f71a9d4 100644 --- a/lib/dns/win32/libdns.def +++ b/lib/dns/win32/libdns.def @@ -407,6 +407,7 @@ dns_name_internalwildcard dns_name_invalidate dns_name_isabsolute dns_name_issubdomain +dns_name_isvalid dns_name_iswildcard dns_name_matcheswildcard dns_name_print