diff --git a/iterator/iter_scrub.c b/iterator/iter_scrub.c index f2f20a5c1..1e3e01330 100644 --- a/iterator/iter_scrub.c +++ b/iterator/iter_scrub.c @@ -294,7 +294,14 @@ synth_cname_rrset(uint8_t** sname, size_t* snamelen, uint8_t* alias, if(ttl_t > MAX_TTL) ttl_t = MAX_TTL; ttl = (uint32_t)ttl_t; sldns_write_uint32(cn->rr_first->ttl_data, ttl); - sldns_write_uint32(rrset->rr_first->ttl_data, ttl); + /* Do NOT write the clamp back into the packet buffer: + * parse_packet already sized every name from the original + * bytes and rdata_copy re-walks them trusting those sizes; + * mutating packet bytes between the walks breaks that + * invariant (compression pointers can target these TTL + * bytes). The DNAME rrset receives the same clamp at store + * time in rdata_copy, so the DNAME and the synthesized + * CNAME still carry equal TTLs in the cache. */ } sldns_write_uint16(cn->rr_first->ttl_data+4, aliaslen); memmove(cn->rr_first->ttl_data+6, alias, aliaslen); diff --git a/util/data/dname.c b/util/data/dname.c index 5370aa6f9..9fbbe1092 100644 --- a/util/data/dname.c +++ b/util/data/dname.c @@ -192,34 +192,34 @@ pkt_dname_len(sldns_buffer* pkt) while(1) { /* read next label */ if(sldns_buffer_remaining(pkt) < 1) - return 0; + goto fail; labellen = sldns_buffer_read_u8(pkt); if(LABEL_IS_PTR(labellen)) { /* compression ptr */ uint16_t ptr; if(sldns_buffer_remaining(pkt) < 1) - return 0; + goto fail; ptr = PTR_OFFSET(labellen, sldns_buffer_read_u8(pkt)); if(ptrcount++ > MAX_COMPRESS_PTRS) - return 0; /* loop! */ + goto fail; /* loop! */ if(sldns_buffer_limit(pkt) <= ptr) - return 0; /* out of bounds! */ + goto fail; /* out of bounds! */ if(!endpos) endpos = sldns_buffer_position(pkt); sldns_buffer_set_position(pkt, ptr); } else { /* label contents */ if(labellen > 0x3f) - return 0; /* label too long */ + goto fail; /* label too long */ len += 1 + labellen; if(len > LDNS_MAX_DOMAINLEN) - return 0; + goto fail; if(labellen == 0) { /* end of dname */ break; } if(sldns_buffer_remaining(pkt) < labellen) - return 0; + goto fail; sldns_buffer_skip(pkt, (ssize_t)labellen); } } @@ -227,6 +227,13 @@ pkt_dname_len(sldns_buffer* pkt) sldns_buffer_set_position(pkt, endpos); return len; +fail: + /* Restore the position on failure too: callers (rdata_copy) compute + * the consumed field length from the buffer position and must not + * see a partial walk of a name that failed to parse. */ + if(endpos) + sldns_buffer_set_position(pkt, endpos); + return 0; } int diff --git a/util/data/msgreply.c b/util/data/msgreply.c index 0beb893c3..71cab7d74 100644 --- a/util/data/msgreply.c +++ b/util/data/msgreply.c @@ -248,6 +248,7 @@ rdata_copy(sldns_buffer* pkt, struct packed_rrset_data* data, uint8_t* to, sldns_pkt_section section) { uint16_t pkt_len; + size_t tolen; uint32_t ttl; const sldns_rr_descriptor* desc; @@ -293,9 +294,13 @@ rdata_copy(sldns_buffer* pkt, struct packed_rrset_data* data, uint8_t* to, (rr->ttl_data - sldns_buffer_begin(pkt) + sizeof(uint32_t))); /* insert decompressed size into rdata len stored in memory */ /* -2 because rdatalen bytes are not included. */ + tolen = rr->size; + if(tolen < 2) + return 0; pkt_len = htons(rr->size - 2); memmove(to, &pkt_len, sizeof(uint16_t)); to += 2; + tolen -= 2; /* read packet rdata len */ pkt_len = sldns_buffer_read_u16(pkt); if(sldns_buffer_remaining(pkt) < pkt_len) @@ -304,16 +309,29 @@ rdata_copy(sldns_buffer* pkt, struct packed_rrset_data* data, uint8_t* to, if(pkt_len > 0 && desc && desc->_dname_count > 0) { int count = (int)desc->_dname_count; int rdf = 0; - size_t len; - size_t oldpos; + size_t len, dlen; + size_t oldpos, newpos; /* decompress dnames. */ while(pkt_len > 0 && count) { switch(desc->_wireformat[rdf]) { case LDNS_RDF_TYPE_DNAME: oldpos = sldns_buffer_position(pkt); - dname_pkt_copy(pkt, to, + dlen = pkt_dname_len(pkt); + if(dlen == 0) + return 0; /* malformed */ + if(dlen > tolen) + return 0; /* alloc mismatch */ + newpos = sldns_buffer_position(pkt); + if(oldpos > newpos) + return 0; /* should have moved forward*/ + sldns_buffer_set_position(pkt, oldpos); + dname_pkt_copy(pkt, to, sldns_buffer_current(pkt)); - to += pkt_dname_len(pkt); + sldns_buffer_set_position(pkt, newpos); + to += dlen; + tolen -= dlen; + if(sldns_buffer_position(pkt)-oldpos > pkt_len) + return 0; /* malformed: walks diverged */ pkt_len -= sldns_buffer_position(pkt)-oldpos; count--; len = 0; @@ -326,9 +344,12 @@ rdata_copy(sldns_buffer* pkt, struct packed_rrset_data* data, uint8_t* to, break; } if(len) { + if(len > tolen) + return 0; /* alloc mismatch */ log_assert(len <= pkt_len); memmove(to, sldns_buffer_current(pkt), len); to += len; + tolen -= len; sldns_buffer_skip(pkt, (ssize_t)len); pkt_len -= len; } @@ -336,8 +357,11 @@ rdata_copy(sldns_buffer* pkt, struct packed_rrset_data* data, uint8_t* to, } } /* copy remaining rdata */ - if(pkt_len > 0) + if(pkt_len > 0) { + if(pkt_len > tolen) + return 0; /* alloc mismatch */ memmove(to, sldns_buffer_current(pkt), pkt_len); + } return 1; }