Skip to content

Cap the DoH response read at 65535 bytes (#761) - #771

Merged
kasnder merged 1 commit into
masterfrom
fix/doh-response-size-cap
Aug 22, 2026
Merged

Cap the DoH response read at 65535 bytes (#761)#771
kasnder merged 1 commit into
masterfrom
fix/doh-response-size-cap

Conversation

@kasnder

@kasnder kasnder commented Aug 22, 2026

Copy link
Copy Markdown
Member

The defect

DnsOverHttpsClient.java:218 (master, 197d4047) reads the DoH response with:

byte[] dnsResponse = responseBody.bytes();

No contentLength() check, no cap. OkHttp imposes no practical limit on
ResponseBody.bytes(), so a hostile or broken DoH endpoint can drive an
unbounded heap allocation inside the always-on VPN process.

Honest scoping

This is edge-configuration hardening, not a default-user bug:

  • DoH is opt-in and defaults to off (preferences.xml:64-67).
  • It additionally requires a hostile or broken endpoint; the default endpoint
    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:

  1. MAX_DOH_RESPONSE_BYTES = 65535 — the DNS-over-TCP framing limit, which a
    legitimate DoH response can never exceed.
  2. Reject on the advertised length first: if responseBody.contentLength()
    exceeds the cap, log and take the failure path without reading the body.
  3. Make the actual read bounded: response.peekBody(MAX + 1) never allocates
    more 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.
  4. Oversized bodies take the existing failure path (continue), exactly
    like the existing length < 12 too-short check — so the retry loop still
    applies and resolve() ultimately returns null. Never truncate-and-parse
    into 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 an onFailedAttempt callback in the
retry 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 with setScreenOff(true) to
disable retries:

  • resolveRejectsOversizedDnsResponse — 65536-byte fixed-length body, exercises
    the contentLength() guard; asserts resolve() returns null and exactly one
    request was made.
  • resolveRejectsOversizedChunkedDnsResponse — 65536-byte chunked body (no
    Content-Length), exercises the bounded peekBody path; same assertions.

Normal-sized success is already covered by the existing
resolveSendsCacheFriendlyGetAndReturnsResponse and 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.

./gradlew :app:compileGithubDebugJavaWithJavac -q                              # exit 0
./gradlew :app:testGithubDebugUnitTest --tests 'net.kollnig.missioncontrol.dns.*'  # exit 0
./gradlew :app:testGithubDebugUnitTest                                          # exit 0

Fixes #761

`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>
@kasnder
kasnder merged commit aaa9e07 into master Aug 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DoH client buffers entire response body without a size cap

1 participant