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); + } }