Skip to content

Commit f570544

Browse files
jandro996devflow.devflow-routing-intake
andauthored
Fix unbounded multipart form-field buffering in Jetty 8 PartHelper (#11876)
fix: bound unbounded multipart form-field buffering in Jetty 8 AppSec advice PartHelper.readPartContent() copied a multipart Part's InputStream into an unbounded ByteArrayOutputStream before any WAF size check applied, allowing a single oversized text field to exhaust heap. Cap the read at Config.get().getAppSecMaxFileContentBytes(), reusing the existing file-content byte cap since there is no dedicated form-field knob. fix: cap number of multipart form fields inspected in Jetty 8 PartHelper extractFormFields() bounded the bytes read per field but not the number of fields processed, leaving total allocation unbounded as O(fields x byteCap). Cap the field count via Config.get().getAppSecMaxFileContentCount(), mirroring the MAX_FILES_TO_INSPECT pattern used by MultipartHelper#extractContents() (PR #11706) for file content. Merge branch 'master' into APMSP-3580 Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent a607d2a commit f570544

2 files changed

Lines changed: 49 additions & 2 deletions

File tree

  • dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static datadog.trace.api.gateway.Events.EVENTS;
44

55
import datadog.appsec.api.blocking.BlockingException;
6+
import datadog.trace.api.Config;
67
import datadog.trace.api.gateway.BlockResponseFunction;
78
import datadog.trace.api.gateway.CallbackProvider;
89
import datadog.trace.api.gateway.Flow;
@@ -107,14 +108,23 @@ public static List<String> extractFilenames(Collection<?> parts) {
107108

108109
/**
109110
* Returns a name→values map of form-field parts (those without a {@code filename=} parameter).
110-
* File-upload parts are skipped to avoid reading potentially large content.
111+
* File-upload parts are skipped to avoid reading potentially large content. Reads up to {@link
112+
* Config#getAppSecMaxFileContentBytes()} bytes per field, up to {@link
113+
* Config#getAppSecMaxFileContentCount()} fields total — same knobs and cap pattern as {@code
114+
* MultipartHelper#extractContents()} uses for file content (PR #11706), reused here since there
115+
* is no dedicated "max form fields" config.
111116
*/
112117
public static Map<String, List<String>> extractFormFields(Collection<?> parts) {
113118
if (parts == null || parts.isEmpty()) {
114119
return Collections.emptyMap();
115120
}
121+
int maxFields = Config.get().getAppSecMaxFileContentCount();
116122
Map<String, List<String>> result = new LinkedHashMap<>();
123+
int count = 0;
117124
for (Object obj : parts) {
125+
if (count >= maxFields) {
126+
break;
127+
}
118128
try {
119129
Part part = (Part) obj;
120130
if (filenameFromPart(part) != null) {
@@ -129,6 +139,7 @@ public static Map<String, List<String>> extractFormFields(Collection<?> parts) {
129139
continue;
130140
}
131141
result.computeIfAbsent(name, k -> new ArrayList<>()).add(value);
142+
count++;
132143
} catch (Exception e) {
133144
log.debug("extractFormFields: skipping malformed part", e);
134145
}
@@ -259,12 +270,20 @@ public static BlockingException fireFilenamesEvent(Collection<?> parts, RequestC
259270

260271
private static String readPartContent(Part part) {
261272
Charset charset = charsetFromContentType(part.getContentType());
273+
// Bound the buffered form-field text by the file-content byte cap. There is no dedicated
274+
// "max form-field bytes" config, so we intentionally reuse getAppSecMaxFileContentBytes():
275+
// this is the only framework that must manually buffer form-field text (Servlet 3.0 has no
276+
// container-side bound), and without a cap a single huge text field could exhaust the heap.
277+
int maxBytes = Config.get().getAppSecMaxFileContentBytes();
262278
try (InputStream is = part.getInputStream()) {
263279
ByteArrayOutputStream baos = new ByteArrayOutputStream();
264280
byte[] buf = new byte[4096];
281+
int total = 0;
265282
int read;
266-
while ((read = is.read(buf)) != -1) {
283+
while (total < maxBytes
284+
&& (read = is.read(buf, 0, Math.min(buf.length, maxBytes - total))) != -1) {
267285
baos.write(buf, 0, read);
286+
total += read;
268287
}
269288
return new String(baos.toByteArray(), charset);
270289
} catch (IOException e) {

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/java/datadog/trace/instrumentation/jetty8/PartHelperTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package datadog.trace.instrumentation.jetty8;
22

33
import static java.util.Arrays.asList;
4+
import static java.util.Arrays.fill;
45
import static java.util.Collections.emptyList;
56
import static java.util.Collections.singletonList;
67
import static org.junit.jupiter.api.Assertions.assertEquals;
78
import static org.junit.jupiter.api.Assertions.assertNull;
89
import static org.mockito.Mockito.mock;
910
import static org.mockito.Mockito.when;
1011

12+
import datadog.trace.api.Config;
1113
import java.io.ByteArrayInputStream;
1214
import java.io.IOException;
1315
import java.nio.charset.Charset;
@@ -236,6 +238,32 @@ void extractFormFieldsDecodesFieldUsingContentTypeCharset() throws IOException {
236238
assertEquals(singletonList("café"), result.get("drink"));
237239
}
238240

241+
@Test
242+
void extractFormFieldsTruncatesFieldExceedingMaxContentBytes() throws IOException {
243+
int maxBytes = Config.get().getAppSecMaxFileContentBytes();
244+
// ASCII value larger than the cap so byte length == char length and truncation is exact.
245+
char[] chars = new char[maxBytes * 2 + 123];
246+
fill(chars, 'a');
247+
String oversized = new String(chars);
248+
List<Part> parts = singletonList(field("big", oversized));
249+
Map<String, List<String>> result = PartHelper.extractFormFields(parts);
250+
List<String> values = result.get("big");
251+
assertEquals(1, values.size());
252+
assertEquals(maxBytes, values.get(0).length());
253+
}
254+
255+
@Test
256+
void extractFormFieldsCapsAtMaxFileContentCount() throws IOException {
257+
int maxFields = Config.get().getAppSecMaxFileContentCount();
258+
int count = maxFields + 1;
259+
Part[] parts = new Part[count];
260+
for (int i = 0; i < count; i++) {
261+
parts[i] = field("field" + i, "value" + i);
262+
}
263+
Map<String, List<String>> result = PartHelper.extractFormFields(asList(parts));
264+
assertEquals(maxFields, result.size());
265+
}
266+
239267
// ── getAllParts ─────────────────────────────────────────────────────────────
240268

241269
@Test

0 commit comments

Comments
 (0)