From 6b93c566feb3e0132b4a77611d82be84e6cfd447 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 27 Jul 2026 15:52:19 +0200 Subject: [PATCH 1/5] resteasy: bound multipart body-map inspection and add UTF-8/text-plain handling Cap collectBodyMap() by total accumulated values (not distinct field names) to close a bypass where a repeated field name could exhaust the limit. Filter to text/plain parts only and default undeclared charsets to UTF-8 for body-map values, matching the Jersey reference. Widen per-part read/header exceptions to Exception with debug logging so one malformed part doesn't abort body/filename/content collection for the whole request. --- ...ultipartFormDataReaderInstrumentation.java | 16 +- .../resteasy/MultipartHelper.java | 83 ++++++- .../test/groovy/MultipartHelperTest.groovy | 235 +++++++++++++++++- 3 files changed, 317 insertions(+), 17 deletions(-) diff --git a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartFormDataReaderInstrumentation.java b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartFormDataReaderInstrumentation.java index 4a435fb0c93..6e5d5542cbc 100644 --- a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartFormDataReaderInstrumentation.java +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartFormDataReaderInstrumentation.java @@ -16,14 +16,10 @@ import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.BiFunction; import net.bytebuddy.asm.Advice; -import org.jboss.resteasy.plugins.providers.multipart.InputPart; import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataInput; @AutoService(InstrumenterModule.class) @@ -67,8 +63,7 @@ public static class ReadFromAdvice { static void after( @Advice.Return final MultipartFormDataInput ret, @ActiveRequestContext RequestContext reqCtx, - @Advice.Thrown(readOnly = false) Throwable t) - throws IOException { + @Advice.Thrown(readOnly = false) Throwable t) { if (ret == null || t != null) { return; } @@ -85,14 +80,7 @@ static void after( } if (callback != null) { - Map> m = new HashMap<>(); - for (Map.Entry> e : ret.getFormDataMap().entrySet()) { - List strings = new ArrayList<>(); - m.put(e.getKey(), strings); - for (InputPart inputPart : e.getValue()) { - strings.add(inputPart.getBodyAsString()); - } - } + Map> m = MultipartHelper.collectBodyMap(ret); Flow flow = callback.apply(reqCtx, m); BlockingException be = diff --git a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java index b9cd5195918..a1a63beb517 100644 --- a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java @@ -6,18 +6,22 @@ import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.http.MultipartContentDecoder; -import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; import org.jboss.resteasy.plugins.providers.multipart.InputPart; import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataInput; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class MultipartHelper { + private static final Logger log = LoggerFactory.getLogger(MultipartHelper.class); + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); @@ -35,6 +39,78 @@ private MultipartHelper() {} GET_HEADERS = m; } + /** + * Builds the {@code server.request.body} map out of the multipart parts. + * + *

Only text/plain parts are collected, matching the Jersey reference: file parts are reported + * separately via {@link #collectFilenames} / {@link #collectFilesContent} and must not also + * consume the body-map budget. The number of collected values is capped by {@link + * #MAX_FILES_TO_INSPECT}. The cap counts the total accumulated values across all field names, not + * the distinct keys: {@code getFormDataMap()} already groups parts by field name, so a per-key + * cap would be trivially bypassed by repeating the same field name on every part. + */ + public static Map> collectBodyMap(MultipartFormDataInput ret) { + Map> bodyMap = new HashMap<>(); + int total = 0; + for (Map.Entry> e : ret.getFormDataMap().entrySet()) { + for (InputPart inputPart : e.getValue()) { + String contentType = contentTypeOf(inputPart); + if (!isTextPlain(contentType)) { + continue; + } + if (total >= MAX_FILES_TO_INSPECT) { + return bodyMap; + } + bodyMap + .computeIfAbsent(e.getKey(), k -> new ArrayList<>()) + .add(readContent(inputPart, contentTypeWithDefaultUtf8(contentType))); + total++; + } + } + return bodyMap; + } + + // No Content-Type header means RESTEasy defaults the part to text/plain (see InputPart's own + // javadoc), so a null contentType is treated as text/plain here as well. + private static boolean isTextPlain(String contentType) { + if (contentType == null) { + return true; + } + int semi = contentType.indexOf(';'); + String mediaType = (semi >= 0 ? contentType.substring(0, semi) : contentType).trim(); + return mediaType.equalsIgnoreCase("text/plain"); + } + + // Used for the body-map/text-field path only: matches Jersey's own getValue(), which decodes + // undeclared-charset text parts as UTF-8 instead of falling back to the JVM platform charset + // (MultipartContentDecoder's default for the filesContent path, kept as-is for parity with the + // other multipart integrations). + private static String contentTypeWithDefaultUtf8(String contentType) { + return MultipartContentDecoder.extractCharset(contentType) == null + ? (contentType == null ? "charset=UTF-8" : contentType + "; charset=UTF-8") + : contentType; + } + + private static String contentTypeOf(InputPart inputPart) { + if (GET_HEADERS == null) { + return null; + } + try { + @SuppressWarnings("unchecked") + Map> headers = (Map>) GET_HEADERS.invoke(inputPart); + if (headers == null) { + return null; + } + List ctHeaders = getHeaderCaseInsensitive(headers, "Content-Type"); + return (ctHeaders != null && !ctHeaders.isEmpty()) ? ctHeaders.get(0) : null; + } catch (Exception e) { + // Reflective getHeaders() call failed (unexpected InputPart implementation): fall back to + // resolving no content-type rather than aborting the whole request's body-map collection. + log.debug("Failed to read multipart part headers via reflection", e); + return null; + } + } + public static List collectFilenames(MultipartFormDataInput ret) { List filenames = new ArrayList<>(); if (GET_HEADERS == null) { @@ -121,7 +197,10 @@ static String readContent(InputPart inputPart, String contentType) { try (InputStream is = inputPart.getBody(InputStream.class, null)) { if (is == null) return ""; return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); - } catch (IOException ignored) { + } catch (Exception e) { + // getBody()/readInputStream() can throw unchecked exceptions too (e.g. a MessageBodyReader + // lookup failure); one bad part must not abort the whole request's body/content collection. + log.debug("Failed to read multipart part content, returning empty string", e); return ""; } } diff --git a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy index ab08b74d151..91f32f695ed 100644 --- a/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy @@ -141,7 +141,7 @@ class MultipartHelperTest extends Specification { MultipartHelper.filenameFromContentDisposition("form-data; filename*=UTF-8''evil.php") == null } - // collectFilesContent + // header fixtures private static MultivaluedMapImpl headers(String cd) { def h = new MultivaluedMapImpl() @@ -149,6 +149,239 @@ class MultipartHelperTest extends Specification { h } + private static MultivaluedMapImpl headers(String cd, String contentType) { + def h = headers(cd) + h.add('Content-Type', contentType) + h + } + + // collectBodyMap + + def "collectBodyMap collects text parts grouped by field name"() { + given: + def p1 = Mock(InputPart) + p1.getHeaders() >> headers('form-data; name="field"') + p1.getBody(_, _) >> new ByteArrayInputStream('value'.bytes) + def p2 = Mock(InputPart) + p2.getHeaders() >> headers('form-data; name="tag"') + p2.getBody(_, _) >> new ByteArrayInputStream('a'.bytes) + def p3 = Mock(InputPart) + p3.getHeaders() >> headers('form-data; name="tag"') + p3.getBody(_, _) >> new ByteArrayInputStream('b'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [p1], 'tag': [p2, p3]] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result == ['field': ['value'], 'tag': ['a', 'b']] + } + + def "collectBodyMap returns an empty map when there are no parts"() { + given: + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> [:] + + expect: + MultipartHelper.collectBodyMap(ret).isEmpty() + } + + def "collectBodyMap decodes using the part declared charset"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="field"', 'text/plain; charset=ISO-8859-1') + part.getBody(_, _) >> new ByteArrayInputStream('café'.getBytes('ISO-8859-1')) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [part]] + + expect: + MultipartHelper.collectBodyMap(ret) == ['field': ['café']] + } + + def "collectBodyMap truncates a value longer than MAX_CONTENT_BYTES"() { + given: + def longValue = 'a' * (MultipartHelper.MAX_CONTENT_BYTES + 100) + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="field"') + part.getBody(_, _) >> new ByteArrayInputStream(longValue.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [part]] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result['field'][0] == 'a' * MultipartHelper.MAX_CONTENT_BYTES + } + + def "collectBodyMap caps total values across distinct field names"() { + given: + def entries = [:] + (1..MultipartHelper.MAX_FILES_TO_INSPECT + 3).each { i -> + def p = Mock(InputPart) + p.getHeaders() >> headers("form-data; name=\"field${i}\"") + p.getBody(_, _) >> new ByteArrayInputStream("value${i}".bytes) + entries["field${i}".toString()] = [p] + } + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> entries + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result.values().sum { it.size() } == MultipartHelper.MAX_FILES_TO_INSPECT + } + + def "collectBodyMap caps total values even when every part reuses the same field name"() { + given: "all parts share a single field name, so the map only ever has one key" + def parts = (1..MultipartHelper.MAX_FILES_TO_INSPECT + 5).collect { i -> + def p = Mock(InputPart) + p.getHeaders() >> headers('form-data; name="same"') + p.getBody(_, _) >> new ByteArrayInputStream("value${i}".bytes) + p + } + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['same': parts] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: "the cap counts total values, not distinct keys" + result.size() == 1 + result['same'].size() == MultipartHelper.MAX_FILES_TO_INSPECT + } + + def "collectBodyMap keeps accumulating values for an already present field below the cap"() { + given: + def parts = (1..3).collect { i -> + def p = Mock(InputPart) + p.getHeaders() >> headers('form-data; name="tag"') + p.getBody(_, _) >> new ByteArrayInputStream("v${i}".bytes) + p + } + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['tag': parts] + + expect: + MultipartHelper.collectBodyMap(ret) == ['tag': ['v1', 'v2', 'v3']] + } + + def "collectBodyMap maps a part to an empty string when reading it fails"() { + given: + def failing = Mock(InputPart) + failing.getHeaders() >> headers('form-data; name="bad"') + failing.getBody(_, _) >> { throw new IOException('boom') } + def ok = Mock(InputPart) + ok.getHeaders() >> headers('form-data; name="good"') + ok.getBody(_, _) >> new ByteArrayInputStream('fine'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['bad': [failing], 'good': [ok]] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result == ['bad': [''], 'good': ['fine']] + noExceptionThrown() + } + + def "collectBodyMap maps a part to an empty string when reading it throws an unchecked exception"() { + given: + def failing = Mock(InputPart) + failing.getHeaders() >> headers('form-data; name="bad"') + failing.getBody(_, _) >> { throw new RuntimeException('no MessageBodyReader') } + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['bad': [failing]] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result == ['bad': ['']] + noExceptionThrown() + } + + def "collectBodyMap treats a part whose getHeaders() throws as having no content-type, without aborting"() { + given: + def broken = Mock(InputPart) + broken.getHeaders() >> { throw new IllegalStateException('boom') } + broken.getBody(_, _) >> new ByteArrayInputStream('value'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [broken]] + + when: + def result = MultipartHelper.collectBodyMap(ret) + + then: + result == ['field': ['value']] + noExceptionThrown() + } + + def "collectBodyMap treats a part whose getHeaders() returns null as having no content-type"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> null + part.getBody(_, _) >> new ByteArrayInputStream('value'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [part]] + + expect: + MultipartHelper.collectBodyMap(ret) == ['field': ['value']] + } + + def "collectBodyMap decodes an undeclared-charset value as UTF-8, not the JVM platform default"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="field"') + part.getBody(_, _) >> new ByteArrayInputStream('café'.getBytes('UTF-8')) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [part]] + + expect: + MultipartHelper.collectBodyMap(ret) == ['field': ['café']] + } + + def "collectBodyMap excludes a non-text/plain part and does not consume the cap for it"() { + given: "a file part and a text part sharing the same request" + def file = Mock(InputPart) + file.getHeaders() >> headers('form-data; name="upload"; filename="a.bin"', 'application/octet-stream') + def text = Mock(InputPart) + text.getHeaders() >> headers('form-data; name="q"') + text.getBody(_, _) >> new ByteArrayInputStream('