Repository navigation
Conversation
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.
devnexen
reviewed
Nov 8, 2026
| #define CHECKCP(n) do { \ | ||
| if (cp + n > end) { \ | ||
| return NULL; \ | ||
| PARSERR_FAIL(); \ |
Member
There was a problem hiding this comment.
The fix itself is correct, I d prefer personally a goto though.
Contributor
Author
There was a problem hiding this comment.
Thanks, updated to a single fail: label at the end of php_parserr() (5bd7c58). No functional change; the resolver-shim cases give the same output as before.
As suggested in review, replace the PARSERR_FAIL() macro with a single fail: label at the end of php_parserr(). No functional change.
devnexen
approved these changes
Nov 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
php_parserr()creates the record array and addshost,classandttlbefore it parses the type-specific data. If that parse fails (aCHECKCP()bound check, a failingdn_expand(), or the CAA length check), it returnsNULLbut leaves the populated array insubarray. The callers inPHP_FUNCTION(dns_get_record)only testZ_TYPE(retval) != IS_UNDEF, so the incomplete array is appended to the result (or to$authns/$addtl) before theNULLends the loop.For a response whose last answer is malformed:
type(andprifor MX) but notarget.rdlength< 4 at the end of the message: the record has onlyhost,classandttl.This change destroys the array on every failure path, as
default:already does for unknown types (zval_ptr_dtor(subarray); ZVAL_UNDEF(subarray);).subarrayis set to UNDEF at the start ofphp_parserr(), so the early failures beforearray_init()are unaffected.There is no memory-safety issue; the impact is low: a resolver or authoritative server sending a malformed response can make
dns_get_record()return entries with missingtype/target/ipkeys.Tested with an
LD_PRELOADshim that replacesres_nsearch()with canned responses (one good A record followed by an MX whose target is the compression pointer0xc0ff; an A record withrdlength2; a CNAME with a bad pointer):No
.phptis included: it would need a resolver mock, which the test suite doesn't have (GH-23798 left out a test for the same reason).Related: GH-22201 (missing
CHECKCPin the A6 branch); with this change that check also drops the partial record. GH-17919 is aboutdns_get_record()error reporting in general.This was found while following up cases from an LLM-Enhanced Static Analysis Engine and verified manually.