Skip to content

Commit a0035e7

Browse files
author
Ankit Prateek
committed
Cap HTTP response body size to bound adversarial-target memory use
The scanner's HTTP egress paths buffered response bodies into memory with no size ceiling on three independent code paths: * common/.../OkHttpHttpClient.java :: parseResponse — used by every detector going through send / sendAsync. Calls okResponse.body().bytes(), which only enforces a 2 GB cap on bodies advertising a Content-Length and that ceiling is bypassed entirely by chunked encoding with no Content-Length header. * common/.../OkHttpHttpClient.java :: sendAsIs — used by the crafted-URL probes that fire against hostile targets. Calls ByteString.readFrom on the raw HttpURLConnection input stream with no ceiling at all. * plugin_server/py/.../requests_http_client.py :: send + _parse_response — uses session.send() with the default stream=False, which buffers the entire body into the Response object before returning, then reads res.content unconditionally. Because Tsunami probes adversarial endpoints by definition, an attacker-controlled target can detect the TsunamiSecurityScanner User-Agent on inbound requests and serve an unbounded chunked response. Each chunk arrives within the read-timeout window so the read timeout never fires, and the body buffer grows until the JVM heap or Python process is exhausted (CWE-770 / CWE-400). The result: a compromised asset can deterministically crash the scanner that is probing it, producing the false-negative "scanned, no findings" outcome an attacker wants from a detection tool. Fix: introduce a configurable max-response-body cap (default 100 MB) enforced at every body-ingestion point. Reads are drained chunk by chunk into a bounded buffer; once the cap is exceeded, the underlying connection is closed and the call fails with IOException / IOError so the affected probe errors out cleanly while the surrounding scan continues. Wiring: * Java: new HttpClientModule.@MaxResponseBodyBytes provider resolves --http-client-max-response-body-mb (CLI) → maxResponseBodyMb (config) → 100 MB default, mirroring the existing timeout flag pattern. New OkHttpHttpClient constructor + builder setter expose the cap; the legacy 6-arg constructor is preserved with the default value to keep external callers source-compatible. * Python: new --max_response_body_mb absl flag plumbed through RequestsHttpClientBuilder.set_max_response_body_bytes into RequestsHttpClient. session.send() now passes stream=True so the body never lands in the Response object before we get a chance to cap it. Tests: * OkHttpHttpClientTest: send / sendAsIs throw IOException when the body exceeds the cap; bodies within the cap pass through unchanged. * requests_http_client_test: send raises IOError on advertised Content-Length over cap, on actual streamed bytes over cap, and passes through small bodies unchanged. Disclosure note: the unbounded reads were originally surfaced by an external security review. The patch is local to the HTTP-client layer; no detector code changes and no detector behavior changes for any response under the cap. Couldn't run gradle/pytest locally (no wrapper checked in, no project-wide pytest harness configured); relying on CI.
1 parent f29c42a commit a0035e7

8 files changed

Lines changed: 333 additions & 7 deletions

File tree

common/src/main/java/com/google/tsunami/common/net/http/HttpClientCliOptions.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,26 @@ public final class HttpClientCliOptions implements CliOption {
6565
description = "User-Agent to use in HTTP requests.")
6666
public String userAgent = HttpClient.TSUNAMI_USER_AGENT;
6767

68+
@Parameter(
69+
names = "--http-client-max-response-body-mb",
70+
description =
71+
"Maximum size in megabytes of an HTTP response body the client will buffer in memory."
72+
+ " Responses larger than this cause the affected probe to fail with an IOException"
73+
+ " while the surrounding scan continues. Defaults to 100 MB.")
74+
Integer maxResponseBodyMb;
75+
6876
@Override
6977
public void validate() {
7078
validateTimeout("--http-client-call-timeout-seconds", callTimeoutSeconds);
7179
validateTimeout("--http-client-connect-timeout-seconds", connectTimeoutSeconds);
7280
validateTimeout("--http-client-read-timeout-seconds", readTimeoutSeconds);
7381
validateTimeout("--http-client-write-timeout-seconds", writeTimeoutSeconds);
82+
if (maxResponseBodyMb != null && maxResponseBodyMb <= 0) {
83+
throw new ParameterException(
84+
String.format(
85+
"--http-client-max-response-body-mb must be positive, received %d.",
86+
maxResponseBodyMb));
87+
}
7488
}
7589

7690
private static void validateTimeout(String flagName, @Nullable Integer value) {

common/src/main/java/com/google/tsunami/common/net/http/HttpClientConfigProperties.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,11 @@ public final class HttpClientConfigProperties {
5050
* more details.
5151
*/
5252
Integer writeTimeoutSeconds;
53+
54+
/**
55+
* Maximum size in megabytes of an HTTP response body the client will buffer in memory. Responses
56+
* larger than this cause the affected probe to fail with an IOException while the surrounding
57+
* scan continues. Defaults to 100 MB when unset.
58+
*/
59+
Integer maxResponseBodyMb;
5360
}

common/src/main/java/com/google/tsunami/common/net/http/HttpClientModule.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,16 @@ HttpClient provideOkHttpHttpClient(
141141
ConnectionFactory connectionFactory,
142142
@LogId String logId,
143143
@ConnectTimeout Duration connectTimeout,
144-
@UserAgent String userAgent) {
144+
@UserAgent String userAgent,
145+
@MaxResponseBodyBytes long maxResponseBodyBytes) {
145146
return new OkHttpHttpClient(
146-
okHttpClient, trustAllCertificates, connectionFactory, logId, connectTimeout, userAgent);
147+
okHttpClient,
148+
trustAllCertificates,
149+
connectionFactory,
150+
logId,
151+
connectTimeout,
152+
userAgent,
153+
maxResponseBodyBytes);
147154
}
148155

149156
@Provides
@@ -271,6 +278,23 @@ String provideUserAgent(HttpClientCliOptions httpClientCliOptions) {
271278
return HttpClient.TSUNAMI_USER_AGENT;
272279
}
273280

281+
@Provides
282+
@MaxResponseBodyBytes
283+
long provideMaxResponseBodyBytes(
284+
HttpClientCliOptions httpClientCliOptions,
285+
HttpClientConfigProperties httpClientConfigProperties) {
286+
Integer megabytes = null;
287+
if (httpClientCliOptions.maxResponseBodyMb != null) {
288+
megabytes = httpClientCliOptions.maxResponseBodyMb;
289+
} else if (httpClientConfigProperties.maxResponseBodyMb != null) {
290+
megabytes = httpClientConfigProperties.maxResponseBodyMb;
291+
}
292+
if (megabytes == null) {
293+
return OkHttpHttpClient.DEFAULT_MAX_RESPONSE_BODY_BYTES;
294+
}
295+
return ((long) megabytes) * 1024L * 1024L;
296+
}
297+
274298
@Qualifier
275299
@Retention(RetentionPolicy.RUNTIME)
276300
@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD})
@@ -326,6 +350,11 @@ String provideUserAgent(HttpClientCliOptions httpClientCliOptions) {
326350
@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD})
327351
@interface UserAgent {}
328352

353+
@Qualifier
354+
@Retention(RetentionPolicy.RUNTIME)
355+
@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD})
356+
@interface MaxResponseBodyBytes {}
357+
329358
/** Builder for {@link HttpClientModule}. */
330359
public static final class Builder {
331360
private static final int DEFAULT_CONNECTION_POOL_MAX_IDLE = 5;

common/src/main/java/com/google/tsunami/common/net/http/OkHttpHttpClient.java

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
import com.google.protobuf.ByteString;
3131
import com.google.tsunami.common.net.http.javanet.ConnectionFactory;
3232
import com.google.tsunami.proto.NetworkService;
33+
import java.io.ByteArrayOutputStream;
3334
import java.io.IOException;
35+
import java.io.InputStream;
3436
import java.net.HttpURLConnection;
3537
import java.time.Duration;
3638
import java.util.List;
@@ -46,6 +48,8 @@
4648
import okhttp3.RequestBody;
4749
import okhttp3.Response;
4850
import okhttp3.ResponseBody;
51+
import okio.Buffer;
52+
import okio.BufferedSource;
4953
import org.checkerframework.checker.nullness.qual.Nullable;
5054

5155
/**
@@ -55,12 +59,19 @@
5559
final class OkHttpHttpClient extends HttpClient {
5660
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
5761

62+
// Default ceiling on response body bytes a single HTTP call may buffer in memory. Targets
63+
// probed by the scanner are by definition adversarial, so an unbounded read is a remote DoS
64+
// primitive (CWE-770 / CWE-400). 100 MB comfortably accommodates any HTML/JSON payload a
65+
// detector should ever need while bounding the worst-case allocation per call.
66+
static final long DEFAULT_MAX_RESPONSE_BODY_BYTES = 100L * 1024 * 1024;
67+
5868
private final OkHttpClient okHttpClient;
5969
private final boolean trustAllCertificates;
6070
private final ConnectionFactory connectionFactory;
6171
private final String logId;
6272
private final Duration connectionTimeout;
6373
private final String userAgent;
74+
private final long maxResponseBodyBytes;
6475

6576
OkHttpHttpClient(
6677
OkHttpClient okHttpClient,
@@ -69,12 +80,31 @@ final class OkHttpHttpClient extends HttpClient {
6980
String logId,
7081
Duration connectionTimeout,
7182
String userAgent) {
83+
this(
84+
okHttpClient,
85+
trustAllCertificates,
86+
connectionFactory,
87+
logId,
88+
connectionTimeout,
89+
userAgent,
90+
DEFAULT_MAX_RESPONSE_BODY_BYTES);
91+
}
92+
93+
OkHttpHttpClient(
94+
OkHttpClient okHttpClient,
95+
boolean trustAllCertificates,
96+
ConnectionFactory connectionFactory,
97+
String logId,
98+
Duration connectionTimeout,
99+
String userAgent,
100+
long maxResponseBodyBytes) {
72101
this.okHttpClient = checkNotNull(okHttpClient);
73102
this.trustAllCertificates = trustAllCertificates;
74103
this.connectionFactory = checkNotNull(connectionFactory);
75104
this.logId = logId;
76105
this.connectionTimeout = connectionTimeout;
77106
this.userAgent = isNullOrEmpty(userAgent) ? TSUNAMI_USER_AGENT : userAgent;
107+
this.maxResponseBodyBytes = maxResponseBodyBytes;
78108
}
79109

80110
/**
@@ -134,7 +164,7 @@ public HttpResponse sendAsIs(HttpRequest httpRequest) throws IOException {
134164
return HttpResponse.builder()
135165
.setStatus(HttpStatus.fromCode(responseCode))
136166
.setHeaders(responseHeadersBuilder.build())
137-
.setBodyBytes(ByteString.readFrom(connection.getInputStream()))
167+
.setBodyBytes(readStreamWithCap(connection.getInputStream(), httpRequest.url()))
138168
.build();
139169
}
140170

@@ -310,7 +340,7 @@ private static RequestBody buildRequestBody(HttpRequest httpRequest) {
310340
mediaType, httpRequest.requestBody().orElse(ByteString.EMPTY).toByteArray());
311341
}
312342

313-
private static HttpResponse parseResponse(Response okResponse) throws IOException {
343+
private HttpResponse parseResponse(Response okResponse) throws IOException {
314344
logger.atInfo().log(
315345
"Received HTTP response with code '%d' for request to '%s'.",
316346
okResponse.code(), okResponse.request().url());
@@ -322,11 +352,65 @@ private static HttpResponse parseResponse(Response okResponse) throws IOExceptio
322352
.setResponseUrl(okResponse.request().url());
323353
if (!okResponse.request().method().equals(HttpMethod.HEAD.name())
324354
&& okResponse.body() != null) {
325-
httpResponseBuilder.setBodyBytes(ByteString.copyFrom(okResponse.body().bytes()));
355+
httpResponseBuilder.setBodyBytes(
356+
readBodyWithCap(okResponse.body(), okResponse.request().url().toString()));
326357
}
327358
return httpResponseBuilder.build();
328359
}
329360

361+
// Reads an OkHttp response body with a hard byte ceiling. Bypasses ResponseBody.bytes()
362+
// because that buffers the entire body and only enforces a 2 GB ceiling on bodies whose
363+
// Content-Length is advertised (a malicious chunked-encoded peer with no Content-Length
364+
// bypasses it entirely).
365+
private ByteString readBodyWithCap(ResponseBody body, String requestUrl) throws IOException {
366+
long advertised = body.contentLength();
367+
if (advertised > maxResponseBodyBytes) {
368+
throw new IOException(
369+
String.format(
370+
"Response body for %s advertised %d bytes, exceeds cap of %d bytes.",
371+
requestUrl, advertised, maxResponseBodyBytes));
372+
}
373+
Buffer sink = new Buffer();
374+
long total = 0L;
375+
try (BufferedSource source = body.source()) {
376+
long n;
377+
while ((n = source.read(sink, 8192L)) != -1L) {
378+
total += n;
379+
if (total > maxResponseBodyBytes) {
380+
sink.clear();
381+
throw new IOException(
382+
String.format(
383+
"Response body for %s exceeds cap of %d bytes (read so far: %d).",
384+
requestUrl, maxResponseBodyBytes, total));
385+
}
386+
}
387+
}
388+
return ByteString.copyFrom(sink.readByteArray());
389+
}
390+
391+
// Reads from a raw InputStream (used by sendAsIs, which goes through HttpURLConnection
392+
// rather than OkHttp). Strictly worse than the OkHttp path absent a cap because the
393+
// underlying ByteString.readFrom drains until EOF with no ceiling whatsoever.
394+
private ByteString readStreamWithCap(InputStream stream, String requestUrl) throws IOException {
395+
ByteArrayOutputStream sink = new ByteArrayOutputStream();
396+
byte[] buffer = new byte[8192];
397+
long total = 0L;
398+
try (InputStream in = stream) {
399+
int read;
400+
while ((read = in.read(buffer)) != -1) {
401+
total += read;
402+
if (total > maxResponseBodyBytes) {
403+
throw new IOException(
404+
String.format(
405+
"Response body for %s exceeds cap of %d bytes (read so far: %d).",
406+
requestUrl, maxResponseBodyBytes, total));
407+
}
408+
sink.write(buffer, 0, read);
409+
}
410+
}
411+
return ByteString.copyFrom(sink.toByteArray());
412+
}
413+
330414
private static HttpHeaders convertHeaders(Headers headers) {
331415
HttpHeaders.Builder headersBuilder = HttpHeaders.builder();
332416
for (int i = 0; i < headers.size(); i++) {
@@ -357,6 +441,7 @@ public static class OkHttpHttpClientBuilder extends Builder<OkHttpHttpClient> {
357441
private String logId;
358442
private Duration connectionTimeout;
359443
private String userAgent;
444+
private long maxResponseBodyBytes;
360445

361446
private OkHttpHttpClientBuilder(OkHttpHttpClient okHttpHttpClient) {
362447
this.okHttpClient = okHttpHttpClient.okHttpClient;
@@ -366,6 +451,7 @@ private OkHttpHttpClientBuilder(OkHttpHttpClient okHttpHttpClient) {
366451
this.logId = okHttpHttpClient.logId;
367452
this.connectionTimeout = okHttpHttpClient.connectionTimeout;
368453
this.userAgent = okHttpHttpClient.userAgent;
454+
this.maxResponseBodyBytes = okHttpHttpClient.maxResponseBodyBytes;
369455
}
370456

371457
@Override
@@ -392,6 +478,12 @@ public OkHttpHttpClientBuilder setUserAgent(String userAgent) {
392478
return this;
393479
}
394480

481+
@CanIgnoreReturnValue
482+
public OkHttpHttpClientBuilder setMaxResponseBodyBytes(long maxResponseBodyBytes) {
483+
this.maxResponseBodyBytes = maxResponseBodyBytes;
484+
return this;
485+
}
486+
395487
@Override
396488
public OkHttpHttpClient build() {
397489
return new OkHttpHttpClient(
@@ -400,7 +492,8 @@ public OkHttpHttpClient build() {
400492
connectionFactory,
401493
logId,
402494
connectionTimeout,
403-
userAgent);
495+
userAgent,
496+
maxResponseBodyBytes);
404497
}
405498
}
406499
}

common/src/test/java/com/google/tsunami/common/net/http/OkHttpHttpClientTest.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,63 @@ protected void configure() {
710710
mockWebServer.shutdown();
711711
}
712712

713+
@Test
714+
public void send_whenResponseBodyExceedsCap_throwsIOException() throws IOException {
715+
String responseBody = "0123456789ABCDEF response body that comfortably exceeds 16 bytes";
716+
mockWebServer.enqueue(
717+
new MockResponse()
718+
.setResponseCode(HttpStatus.OK.code())
719+
.setHeader(CONTENT_TYPE, MediaType.PLAIN_TEXT_UTF_8.toString())
720+
.setBody(responseBody));
721+
mockWebServer.start();
722+
723+
HttpClient cappedClient = buildCappedClient(httpClient, 16L);
724+
725+
String requestUrl = mockWebServer.url("/test/get-oversized").toString();
726+
IOException thrown =
727+
assertThrows(
728+
IOException.class,
729+
() -> cappedClient.send(get(requestUrl).withEmptyHeaders().build()));
730+
assertThat(thrown).hasMessageThat().contains("exceeds cap of 16 bytes");
731+
}
732+
733+
@Test
734+
public void sendAsIs_whenResponseBodyExceedsCap_throwsIOException() throws IOException {
735+
mockWebServer.setDispatcher(new SendAsIsTestDispatcher());
736+
mockWebServer.start();
737+
738+
HttpClient cappedClient = buildCappedClient(httpClient, 8L);
739+
740+
HttpUrl baseUrl = mockWebServer.url("/");
741+
String requestUrl =
742+
new URL(baseUrl.scheme(), baseUrl.host(), baseUrl.port(), "/send-as-is/oversized")
743+
.toString();
744+
745+
IOException thrown =
746+
assertThrows(
747+
IOException.class,
748+
() -> cappedClient.sendAsIs(get(requestUrl).withEmptyHeaders().build()));
749+
assertThat(thrown).hasMessageThat().contains("exceeds cap of 8 bytes");
750+
}
751+
752+
@Test
753+
public void send_whenResponseBodyWithinCap_returnsExpectedHttpResponse() throws IOException {
754+
String responseBody = "tiny";
755+
mockWebServer.enqueue(
756+
new MockResponse()
757+
.setResponseCode(HttpStatus.OK.code())
758+
.setHeader(CONTENT_TYPE, MediaType.PLAIN_TEXT_UTF_8.toString())
759+
.setBody(responseBody));
760+
mockWebServer.start();
761+
762+
HttpClient cappedClient = buildCappedClient(httpClient, 16L);
763+
764+
String requestUrl = mockWebServer.url("/test/within-cap").toString();
765+
HttpResponse response = cappedClient.send(get(requestUrl).withEmptyHeaders().build());
766+
767+
assertThat(response.bodyBytes()).hasValue(ByteString.copyFrom(responseBody, UTF_8));
768+
}
769+
713770
@Test
714771
public void send_default_userAgent() throws IOException, InterruptedException {
715772
String responseBody = "test response";
@@ -761,6 +818,16 @@ protected void configure() {
761818
assertThat(mockWebServer.takeRequest().getHeader(USER_AGENT)).isEqualTo(userAgentOverride);
762819
}
763820

821+
// Returns a copy of the supplied client whose response-body cap is set to {@code capBytes}.
822+
// The test only exercises the OkHttp builder, but the cast is safe because the injected
823+
// HttpClient is always backed by OkHttpHttpClientBuilder in this module.
824+
@SuppressWarnings("unchecked")
825+
private static HttpClient buildCappedClient(HttpClient client, long capBytes) {
826+
OkHttpHttpClient.OkHttpHttpClientBuilder builder =
827+
(OkHttpHttpClient.OkHttpHttpClientBuilder) (HttpClient.Builder<?>) client.modify();
828+
return builder.setMaxResponseBodyBytes(capBytes).build();
829+
}
830+
764831
private MockWebServer startMockWebServerWithSsl(InetAddress serverAddress)
765832
throws GeneralSecurityException, IOException {
766833
MockWebServer mockWebServer = new MockWebServer();

0 commit comments

Comments
 (0)