From 123d85d5ef395c64c8c35eec36d295070839622e Mon Sep 17 00:00:00 2001 From: Martin Mirchev Date: Sat, 7 Nov 2026 22:24:21 +0800 Subject: [PATCH 1/2] ext/standard/dns.c: drop partially built records in php_parserr() php_parserr() creates the record array and adds host, class and ttl before it parses the type-specific data. When that parse fails (a CHECKCP() bound check, a failing dn_expand(), or the CAA length check), it returned NULL but left the populated array in subarray, and dns_get_record() appended it to the result, since the callers only test Z_TYPE(retval) != IS_UNDEF. A malformed record therefore showed up as an entry with missing type/target/ip keys. Destroy the array on every failure path, as the default: case already does for unknown types. --- ext/standard/dns.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/ext/standard/dns.c b/ext/standard/dns.c index 64301ed04161..60f578ebc5ca 100644 --- a/ext/standard/dns.c +++ b/ext/standard/dns.c @@ -439,9 +439,16 @@ PHP_FUNCTION(dns_check_record) #ifdef HAVE_FULL_DNS_FUNCS +/* Drop the partially built record, so that the caller does not publish it. */ +#define PARSERR_FAIL() do { \ + zval_ptr_dtor(subarray); \ + ZVAL_UNDEF(subarray); \ + return NULL; \ +} while (0) + #define CHECKCP(n) do { \ if (cp + n > end) { \ - return NULL; \ + PARSERR_FAIL(); \ } \ } while (0) @@ -528,7 +535,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ } n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "target", name); @@ -565,7 +572,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_stringl(subarray, "tag", (char*)cp, n); cp += n; if ( (size_t) dlen < ((size_t)n) + 2 ) { - return NULL; + PARSERR_FAIL(); } n = dlen - n - 2; CHECKCP(n); @@ -608,13 +615,13 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_string(subarray, "type", "SOA"); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) -2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "mname", name); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) -2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "rname", name); @@ -735,7 +742,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ if (cp < p + dlen) { n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "chain", name); @@ -752,7 +759,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_long(subarray, "port", n); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "target", name); @@ -788,7 +795,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - return NULL; + PARSERR_FAIL(); } cp += n; add_assoc_string(subarray, "replacement", name); From 5bd7c58e87c33c770676a0d3d4205ca405f6099b Mon Sep 17 00:00:00 2001 From: Martin Mirchev Date: Tue, 10 Nov 2026 11:41:41 +0800 Subject: [PATCH 2/2] ext/standard/dns.c: use a goto for the php_parserr() failure path As suggested in review, replace the PARSERR_FAIL() macro with a single fail: label at the end of php_parserr(). No functional change. --- ext/standard/dns.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/ext/standard/dns.c b/ext/standard/dns.c index 60f578ebc5ca..9028accb3fd5 100644 --- a/ext/standard/dns.c +++ b/ext/standard/dns.c @@ -439,16 +439,9 @@ PHP_FUNCTION(dns_check_record) #ifdef HAVE_FULL_DNS_FUNCS -/* Drop the partially built record, so that the caller does not publish it. */ -#define PARSERR_FAIL() do { \ - zval_ptr_dtor(subarray); \ - ZVAL_UNDEF(subarray); \ - return NULL; \ -} while (0) - #define CHECKCP(n) do { \ if (cp + n > end) { \ - PARSERR_FAIL(); \ + goto fail; \ } \ } while (0) @@ -535,7 +528,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ } n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "target", name); @@ -572,7 +565,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_stringl(subarray, "tag", (char*)cp, n); cp += n; if ( (size_t) dlen < ((size_t)n) + 2 ) { - PARSERR_FAIL(); + goto fail; } n = dlen - n - 2; CHECKCP(n); @@ -615,13 +608,13 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_string(subarray, "type", "SOA"); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) -2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "mname", name); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) -2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "rname", name); @@ -742,7 +735,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ if (cp < p + dlen) { n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "chain", name); @@ -759,7 +752,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ add_assoc_long(subarray, "port", n); n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "target", name); @@ -795,7 +788,7 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ n = dn_expand(answer->qb2, end, cp, name, (sizeof name) - 2); if (n < 0) { - PARSERR_FAIL(); + goto fail; } cp += n; add_assoc_string(subarray, "replacement", name); @@ -808,6 +801,12 @@ static uint8_t *php_parserr(uint8_t *cp, uint8_t *end, querybuf *answer, int typ } return cp; + +fail: + /* Drop the partially built record, so that the caller does not publish it. */ + zval_ptr_dtor(subarray); + ZVAL_UNDEF(subarray); + return NULL; } /* }}} */