diff --git a/testcode/unitverify.c b/testcode/unitverify.c index fcf2e2ffe..eb1f43c3c 100644 --- a/testcode/unitverify.c +++ b/testcode/unitverify.c @@ -510,6 +510,60 @@ nsec3_hash_test(const char* fname) sldns_buffer_free(buf); } +/** Test the rrset_canonicalize_to_buffer function to see if the + * size of canon_owner name is properly checked for. */ +static void +canon_owner_buf_test(void) +{ + struct regional* region; + sldns_buffer* buf; + struct ub_packed_rrset_key k; + struct packed_rrset_data d; + size_t rr_len[2]; + time_t rr_ttl[2]; + uint8_t* rr_data[2]; + int ret; + unit_show_func("validator/val_sigcrypt.c", + "rrset_canonicalize_to_buffer"); + region = regional_create(); + if(!region) + fatal_exit("out of memory"); + /* Purposefully a very small buffer, to overflow it */ + buf = sldns_buffer_new(28); + if(!buf) + fatal_exit("out of memory"); + + /* An RRset to canonicalize. The buffer is made smaller, so + * it can fail on bounds checks. */ + memset(&d, 0, sizeof(d)); + d.ttl = 3600; + d.count = 1; + d.rrsig_count = 1; + d.rr_len = rr_len; + d.rr_ttl = rr_ttl; + d.rr_data = rr_data; + rr_len[0] = 18; + rr_len[1] = 36; + rr_ttl[0] = 3600; + rr_ttl[1] = 3600; + rr_data[0] = (uint8_t*)"\x00\x10\x0Fzzaaaaaaaaaaaaa"; + rr_data[1] = (uint8_t*)"\x00\x24\x00\x06\x08\x3\x01\x02\x03\x04\x01\x02\x03\x04\x01\x02\x03\x04\x12\x34\x03zzz\x00zzaaaaaaaaaaa"; + + memset(&k, 0, sizeof(k)); + k.rk.dname = (uint8_t*) "\x0f" "aaaaaaaaaaaaaaa" "\x00"; + k.rk.dname_len = 17; + k.rk.type = htons(LDNS_RR_TYPE_TXT); + k.rk.rrset_class = htons(LDNS_RR_CLASS_IN); + k.entry.data = &d; + + /* There should be no buffer overflow, assertion failure, here */ + ret = rrset_canonicalize_to_buffer(region, buf, &k); + unit_assert(ret == 0); + + regional_destroy(region); + sldns_buffer_free(buf); +} + #define xstr(s) str(s) #define str(s) #s @@ -724,4 +778,5 @@ verify_test(void) #endif nsectest(); nsec3_hash_test(SRCDIRSTR "/testdata/test_nsec3_hash.1"); + canon_owner_buf_test(); } diff --git a/validator/val_sigcrypt.c b/validator/val_sigcrypt.c index 4139cc1fe..cae470efd 100644 --- a/validator/val_sigcrypt.c +++ b/validator/val_sigcrypt.c @@ -1308,15 +1308,32 @@ rrset_canonical(struct regional* region, sldns_buffer* buf, } sldns_buffer_clear(buf); + if(sldns_buffer_remaining(buf) < siglen || siglen < 18+1) { + verbose(VERB_ALGO, "verify: failed to canonicalize, " + "rrset too big"); + return 0; + } sldns_buffer_write(buf, sig, siglen); /* canonicalize signer name */ canon_dname_tolower(sldns_buffer_begin(buf)+18, sldns_buffer_current(buf)); + + if(sldns_buffer_remaining(buf) < k->rk.dname_len+2) { + /* Check if the first can_owner name can fit in the buffer. + * The length is k->rk.dname_len or k->rk.dname_len+2 + * if it has '*.' in prefixed. Checks the upper bound, + * also realistically the rest of the rrtype, rrclass, origttl, + * rdata and so on has to be inserted, so that extra space has + * to be there. */ + verbose(VERB_ALGO, "verify: failed to canonicalize, " + "rrset too big"); + return 0; + } RBTREE_FOR(walk, struct canon_rr*, (*sortree)) { /* see if there is enough space left in the buffer */ if(sldns_buffer_remaining(buf) < can_owner_len + 2 + 2 + 4 + d->rr_len[walk->rr_idx]) { - log_err("verify: failed to canonicalize, " + verbose(VERB_ALGO, "verify: failed to canonicalize, " "rrset too big"); return 0; } @@ -1325,6 +1342,13 @@ rrset_canonical(struct regional* region, sldns_buffer* buf, sldns_buffer_write(buf, can_owner, can_owner_len); else insert_can_owner(buf, k, sig, &can_owner, &can_owner_len); + /* Check again, if the rdata can fit in the buffer */ + if(sldns_buffer_remaining(buf) < 2 + 2 + 4 + + d->rr_len[walk->rr_idx]) { + verbose(VERB_ALGO, "verify: failed to canonicalize, " + "rrset too big"); + return 0; + } sldns_buffer_write(buf, &k->rk.type, 2); sldns_buffer_write(buf, &k->rk.rrset_class, 2); sldns_buffer_write(buf, sig+4, 4); @@ -1376,11 +1400,17 @@ rrset_canonicalize_to_buffer(struct regional* region, sldns_buffer* buf, canonical_sort(k, d, sortree, rrs); sldns_buffer_clear(buf); + if(sldns_buffer_remaining(buf) < k->rk.dname_len) { + /* Check if the first can_owner name can fit in the buffer. */ + verbose(VERB_ALGO, "verify: failed to canonicalize, " + "rrset too big"); + return 0; + } RBTREE_FOR(walk, struct canon_rr*, sortree) { /* see if there is enough space left in the buffer */ if(sldns_buffer_remaining(buf) < can_owner_len + 2 + 2 + 4 + d->rr_len[walk->rr_idx]) { - log_err("verify: failed to canonicalize, " + verbose(VERB_ALGO, "verify: failed to canonicalize, " "rrset too big"); return 0; } @@ -1393,6 +1423,13 @@ rrset_canonicalize_to_buffer(struct regional* region, sldns_buffer* buf, query_dname_tolower(can_owner); can_owner_len = k->rk.dname_len; } + /* Check again, if the rdata can fit in the buffer */ + if(sldns_buffer_remaining(buf) < 2 + 2 + 4 + + d->rr_len[walk->rr_idx]) { + verbose(VERB_ALGO, "verify: failed to canonicalize, " + "rrset too big"); + return 0; + } sldns_buffer_write(buf, &k->rk.type, 2); sldns_buffer_write(buf, &k->rk.rrset_class, 2); sldns_buffer_write_u32(buf, d->rr_ttl[walk->rr_idx]);