From d823d8e9cde08ab05a1df521e5c87293588f6f70 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 3 Jun 2026 23:26:41 +0100 Subject: [PATCH] Fix OOB-read crash in get_rdata(). get_rdata() contains another instance of a crash caused by a false value of RDLEN in a resource-record. An RR containing name which is longer than the rdlen of the RR can get get_rdata into a state where it calculates the length of the data following the name as a negative number which is then promoted to a very large positive number, and causing SIGSEGV. This can be triggered by crafted packets from malicous DNS servers, but only when DNSSEC validation is enabled. The attack packets do not have to be correctly DNSSEC signed. Thanks to Tristan G. for spotting this problem. Also fix error handling for RRs with a fixed size fields in the specification where the rrsize indicates that there are zero bytes in that position. --- src/dnssec.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 4c37d0c..71c9bac 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -177,7 +177,7 @@ static int get_rdata(struct dns_header *header, size_t plen, struct rdata_state if ((state->c = state->end - state->ip) != 0) { state->op = state->ip; - state->ip = state->end;; + state->ip = state->end; } else return 0; @@ -189,24 +189,28 @@ static int get_rdata(struct dns_header *header, size_t plen, struct rdata_state if (d == (u16)0) { /* domain-name, canonicalise */ - int len; + /* If the name is malformed, or runs beyond the end of the RR, abort. + under no circumstances should state->ip be beyond state->end since + that will cause the run-out code above to turn the negative + calculted length into a very large postive value for state->c + and overrun the buffer. */ if (!extract_name(header, plen, &state->ip, state->buff, EXTR_NAME_EXTRACT, 0) || - (len = to_wire(state->buff)) == 0) - continue; - - state->c = len; + state->ip > state->end) + return 0; + + /* to_wire() always returns length > 0 */ + state->c = to_wire(state->buff); state->op = (unsigned char *)state->buff; } else { - /* plain data preceding a domain-name, don't run off the end of the data */ - if ((state->end - state->ip) < d) - d = state->end - state->ip; - - if (d == 0) - continue; - + /* Plain data preceding a domain-name. + Handle short RR, if there are no bytes left, return finished. */ + if ((state->end - state->ip) < d && + (d = state->end - state->ip) == 0) + return 0; + state->op = state->ip; state->c = d; state->ip += d;