Cap the DoH response read at 65535 bytes (#761) - #771
Merged
Conversation
`resolve()` passed the whole DoH response body to `responseBody.bytes()` with no length check. OkHttp imposes no practical limit, so a hostile or broken endpoint could drive an unbounded allocation inside the always-on VPN process. Reject on the advertised `contentLength()` before reading anything, and make the read itself bounded via `peekBody(65536)` so a chunked body with no advertised length cannot allocate without limit either. 65535 bytes is the DNS-over-TCP framing limit, so a legitimate DoH response can never exceed it. Oversized bodies take the existing failure path (`continue`), the same as the existing too-short check — never truncate-and-parse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 22, 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.
The defect
DnsOverHttpsClient.java:218(master,197d4047) reads the DoH response with:No
contentLength()check, no cap. OkHttp imposes no practical limit onResponseBody.bytes(), so a hostile or broken DoH endpoint can drive anunbounded heap allocation inside the always-on VPN process.
Honest scoping
This is edge-configuration hardening, not a default-user bug:
preferences.xml:64-67).is Quad9.
It is worth doing anyway because the fix is ~13 lines, adds no user-facing
surface, and carries near-zero risk.
The change
At the body-read site only:
MAX_DOH_RESPONSE_BYTES = 65535— the DNS-over-TCP framing limit, which alegitimate DoH response can never exceed.
responseBody.contentLength()exceeds the cap, log and take the failure path without reading the body.
response.peekBody(MAX + 1)never allocatesmore than 64 KiB + 1, so a chunked body with no advertised length cannot
allocate without limit either. If the peeked array still exceeds the cap,
reject.
continue), exactlylike the existing
length < 12too-short check — so the retry loop stillapplies and
resolve()ultimately returnsnull. Never truncate-and-parseinto a malformed DNS message.
For a normal-sized body the peek reads the source to EOF, so the exchange
still completes and connection reuse is unaffected.
Overlap with #768
PR #768 also edits
resolve()— it adds anonFailedAttemptcallback in theretry loop around this very call — and is pending a rebase. This change is
deliberately confined to the body-read site: no loop refactor, no renames, no
reordering, so the two should merge cleanly. Whichever lands second just needs
the new guard block and the callback to sit side by side.
Test evidence
Two new tests in
DnsOverHttpsClientTest, both withsetScreenOff(true)todisable retries:
resolveRejectsOversizedDnsResponse— 65536-byte fixed-length body, exercisesthe
contentLength()guard; assertsresolve()returnsnulland exactly onerequest was made.
resolveRejectsOversizedChunkedDnsResponse— 65536-byte chunked body (noContent-Length), exercises the boundedpeekBodypath; same assertions.Normal-sized success is already covered by the existing
resolveSendsCacheFriendlyGetAndReturnsResponseand the caching/TTL tests,which all still pass.
Verified both new tests fail on master without the fix (
18 tests completed, 2 failed) and pass with it.Fixes #761