lib/dnssec: fixed missing checks in label skipping

skipping over last/root label returns a pointer to a memory after domain name, this is unsafe

diff --git a/lib/dnssec/nsec.c b/lib/dnssec/nsec.c
index 41f57ca3888d9e106f290324a37aa7f7f660b2c3..70107fa757030156c149cfe2a2b74d6e883dc480 100644 (file)
--- a/lib/dnssec/nsec.c
+++ b/lib/dnssec/nsec.c
@@ -118,7 +118,7 @@ static int name_error_response_check_rr(int *flags, const knot_rrset_t *nsec,
        uint8_t namebuf[KNOT_DNAME_MAXLEN];
        knot_dname_to_wire(namebuf, name, sizeof(namebuf));
        knot_dname_t *ptr = namebuf;
-       while (*ptr != '\0') {
+       while (ptr[0]) {
                /* Remove leftmost label and replace it with '\1*'. */
                ptr = (uint8_t *) knot_wire_next_label(ptr, NULL);
                *(--ptr) = '*';

diff --git a/lib/dnssec/nsec3.c b/lib/dnssec/nsec3.c
index a58a0809eab2702f1dbe373d11d942e0e7a2fcc8..d890e6fdbc21d68ff8983d0e1d8744de6f4a7eeb 100644 (file)
--- a/lib/dnssec/nsec3.c
+++ b/lib/dnssec/nsec3.c
@@ -144,10 +144,16 @@ static int closest_encloser_match(int *flags, const knot_rrset_t *nsec3,
                goto fail;
        }
 
+       /* Root label has no encloser */
+       if (!name[0]) {
+               ret = kr_error(ENOENT);
+               goto fail;
+       }
+
        const knot_dname_t *encloser = knot_wire_next_label(name, NULL);
        *skipped = 1;
 
-       do {
+       while(encloser) {
                ret = hash_name(&name_hash, &params, encloser);
                if (ret != 0) {
                        goto fail;
@@ -162,9 +168,11 @@ static int closest_encloser_match(int *flags, const knot_rrset_t *nsec3,
 
                dnssec_binary_free(&name_hash);
 
+               if (!encloser[0])
+                       break;
                encloser = knot_wire_next_label(encloser, NULL);
                ++(*skipped);
-       } while (encloser && (encloser[0] != '\0'));
+       }
 
        ret = kr_ok();
 
@@ -398,6 +406,7 @@ static int closest_encloser_proof(const knot_pkt_t *pkt, knot_section_t section_
                --skipped;
                next_closer = sname;
                for (unsigned j = 0; j < skipped; ++j) {
+                       assert(next_closer[0]);
                        next_closer = knot_wire_next_label(next_closer, NULL);
                }
                for (unsigned j = 0; j < sec->count; ++j) {
@@ -421,7 +430,7 @@ static int closest_encloser_proof(const knot_pkt_t *pkt, knot_section_t section_
        }
 
        if ((flags & FLG_CLOSEST_PROVABLE_ENCLOSER) && (flags & FLG_NAME_COVERED) && next_closer) {
-               if (encloser_name) {
+               if (encloser_name && next_closer[0]) {
                        *encloser_name = knot_wire_next_label(next_closer, NULL);
                }
                if (matching_ecloser_nsec3) {
@@ -680,6 +689,7 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_
 
        /* Compute the next closer name. */
        for (int i = 0; i < trim_to_next; ++i) {
+               assert(sname[0]);
                sname = knot_wire_next_label(sname, NULL);
        }
 

diff --git a/lib/dnssec/signature.c b/lib/dnssec/signature.c
index 34773fe4ff2dd4ab2d52f67c25a4881d96df1c23..35d82ecae98623b5658a5f29cf374d69f50a2eb3 100644 (file)
--- a/lib/dnssec/signature.c
+++ b/lib/dnssec/signature.c
@@ -198,6 +198,7 @@ static int sign_ctx_add_records(dnssec_sign_ctx_t *ctx, const knot_rrset_t *cove
        if (trim_labels > 0) {
                /**/
                for (int i = 0; i < trim_labels; ++i) {
+                       assert(owner[0]);
                        owner = (uint8_t *) knot_wire_next_label(owner, NULL);
                }
                *(--owner) = '*';

diff --git a/lib/resolve.c b/lib/resolve.c
index ace1e296aa197ccb4553aef54c96eb0e746dc3e5..2fe1b96ee032173bdd3e00590b28ea093e095a5e 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -93,7 +93,7 @@ static inline size_t layer_id(struct kr_request *req, const struct knot_layer_ap
 static void randomized_qname_case(knot_dname_t *qname, uint32_t secret)
 {
        unsigned k = 0;
-       while (*qname != '\0') {
+       while (qname[0]) {
                for (unsigned i = *qname; i--;) {
                        int chr = qname[i + 1];
                        if (isalpha(chr)) {
@@ -132,15 +132,18 @@ static void check_empty_nonterms(struct kr_query *qry, knot_pkt_t *pkt, struct k
 
        const knot_dname_t *target = qry->sname;
        const knot_dname_t *cut_name = qry->zone_cut.name;
+       if (!target || !cut_name)
+               return;
+
        struct kr_cache_entry *entry = NULL;
        /* @note: The non-terminal must be direct child of zone cut (e.g. label distance <= 2),
         *        otherwise this would risk leaking information to parent if the NODATA TTD > zone cut TTD. */
-       size_t labels = knot_dname_labels(target, NULL) - knot_dname_labels(cut_name, NULL);
-       while (labels > 2) {
+       int labels = knot_dname_labels(target, NULL) - knot_dname_labels(cut_name, NULL);
+       while (target[0] && labels > 2) {
                target = knot_wire_next_label(target, NULL);
                --labels;
        }
-       for (size_t i = 0; i < labels; ++i) {
+       for (int i = 0; i < labels; ++i) {
                int ret = kr_cache_peek(txn, KR_CACHE_PKT, target, KNOT_RRTYPE_NS, &entry, &timestamp);
                if (ret == 0) { /* Either NXDOMAIN or NODATA, start here. */
                        /* @todo We could stop resolution here for NXDOMAIN, but we can't because of broken CDNs */
@@ -148,6 +151,7 @@ static void check_empty_nonterms(struct kr_query *qry, knot_pkt_t *pkt, struct k
                        kr_make_query(qry, pkt);
                        return;
                }
+               assert(target[0]);
                target = knot_wire_next_label(target, NULL);
        }
 }