From c228b39fc1793f93261e15c6716cde8722d56f82 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Sat, 22 Aug 2026 12:06:28 +0200 Subject: [PATCH] Cap the DoH response read at 65535 bytes (#761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- .../dns/DnsOverHttpsClient.java | 13 +++++- .../dns/DnsOverHttpsClientTest.java | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java index 2bc9e310..05662a86 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java +++ b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java @@ -63,6 +63,7 @@ public class DnsOverHttpsClient { private static final int WRITE_TIMEOUT_MS = 5000; private static final long HTTP_CACHE_MAX_BYTES = 2L * 1024L * 1024L; private static final int MAX_GET_URL_LENGTH = 2048; + private static final int MAX_DOH_RESPONSE_BYTES = 65535; private static final ExecutorService SHUTDOWN_EXECUTOR = Executors.newSingleThreadExecutor(r -> { Thread thread = new Thread(r, "DoH-shutdown"); thread.setDaemon(true); @@ -224,7 +225,17 @@ public byte[] resolve(@NonNull byte[] dnsQuery) { continue; } - byte[] dnsResponse = responseBody.bytes(); + long contentLength = responseBody.contentLength(); + if (contentLength > MAX_DOH_RESPONSE_BYTES) { + Log.w(TAG, "DoH response too large: " + contentLength + " bytes"); + continue; + } + + byte[] dnsResponse = response.peekBody(MAX_DOH_RESPONSE_BYTES + 1L).bytes(); + if (dnsResponse.length > MAX_DOH_RESPONSE_BYTES) { + Log.w(TAG, "DoH response too large: " + dnsResponse.length + " bytes"); + continue; + } if (dnsResponse.length < 12) { Log.w(TAG, "DoH response too short: " + dnsResponse.length + " bytes"); continue; diff --git a/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java b/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java index a7357c06..6508e351 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java +++ b/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java @@ -130,6 +130,34 @@ public void resolveRetriesInvalidShortDnsResponse() { assertEquals(2, server.getRequestCount()); } + @Test + public void resolveRejectsOversizedDnsResponse() { + DnsOverHttpsClient.setScreenOff(true); + try { + server.enqueue(dnsResponse(200, responseOfLength(65536))); + + assertNull(client().resolve(QUERY)); + + assertEquals(1, server.getRequestCount()); + } finally { + DnsOverHttpsClient.setScreenOff(false); + } + } + + @Test + public void resolveRejectsOversizedChunkedDnsResponse() { + DnsOverHttpsClient.setScreenOff(true); + try { + server.enqueue(chunkedDnsResponse(200, responseOfLength(65536))); + + assertNull(client().resolve(QUERY)); + + assertEquals(1, server.getRequestCount()); + } finally { + DnsOverHttpsClient.setScreenOff(false); + } + } + @Test public void normalizeTransactionIdUsesZeroWithoutMutatingQuery() { byte[] query = new byte[]{0x12, 0x34, 0x01, 0x00}; @@ -255,4 +283,16 @@ private static MockResponse dnsResponse(int status, byte[] body) { .body(new Buffer().write(body)) .build(); } + + private static MockResponse chunkedDnsResponse(int status, byte[] body) { + return new MockResponse.Builder() + .code(status) + .addHeader("Content-Type", "application/dns-message") + .chunkedBody(new Buffer().write(body), 1024) + .build(); + } + + private static byte[] responseOfLength(int length) { + return Arrays.copyOf(RESPONSE, length); + } }