From 37e7d097033443104a5ffb7e8f2da5382824d297 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 16 Apr 2026 16:12:34 +0200 Subject: [PATCH 1/7] feat(appsec): expose uploaded file content as WAF address server.request.body.files_content Introduces a new AppSec WAF address `server.request.body.files_content` (`List`) that exposes the content of each uploaded file in a multipart/form-data request. Entries correspond positionally to the existing `server.request.body.filenames` address. Content is capped at 4 096 bytes per file (ISO-8859-1) to keep memory usage bounded. Changes: - KnownAddresses: add REQUEST_FILES_CONTENT + forName() case - Events: add requestFilesContent event (ID 31); FILE_WRITTEN bumped to 32 - InstrumentationGateway: register the new BiFunction case - GatewayBridge: add onRequestFilesContent handler + DATA_DEPENDENCIES entry - CommonsFileUploadAppSecModule: after firing filenames, fire content (skipped when the filenames event already blocked the request) - Unit tests: GatewayBridgeSpecification, GatewayBridgeIGRegistrationSpecification, KnownAddressesSpecificationForkedTest - Smoke test: 'block request based on malicious file upload content' verifies end-to-end blocking via a custom WAF rule on the new address Closes APPSEC-61875 --- .../appsec/event/data/KnownAddresses.java | 10 +++ .../datadog/appsec/gateway/GatewayBridge.java | 32 +++++++++ ...ownAddressesSpecificationForkedTest.groovy | 3 +- ...ayBridgeIGRegistrationSpecification.groovy | 11 +++ .../gateway/GatewayBridgeSpecification.groovy | 38 +++++++++- .../CommonsFileUploadAppSecModule.java | 72 ++++++++++++++++--- .../appsec/SpringBootSmokeTest.groovy | 52 ++++++++++++++ .../datadog/trace/api/gateway/Events.java | 14 +++- .../api/gateway/InstrumentationGateway.java | 2 + 9 files changed, 221 insertions(+), 13 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index 4a5a66e0074..dd136659220 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -74,6 +74,14 @@ public interface KnownAddresses { Address REQUEST_COMBINED_FILE_SIZE = new Address<>("server.request.body.combined_file_size"); + /** + * Contains the content of each uploaded file in a multipart/form-data request. Each entry in the + * list corresponds positionally to {@link #REQUEST_FILES_FILENAMES}. Content is truncated to a + * maximum size to avoid excessive memory usage. Available only on inspected multipart/form-data + * requests. + */ + Address> REQUEST_FILES_CONTENT = new Address<>("server.request.body.files_content"); + /** * The parsed query string. * @@ -205,6 +213,8 @@ static Address forName(String name) { return REQUEST_FILES_FILENAMES; case "server.request.body.combined_file_size": return REQUEST_COMBINED_FILE_SIZE; + case "server.request.body.files_content": + return REQUEST_FILES_CONTENT; case "server.request.query": return REQUEST_QUERY; case "server.request.headers.no_cookies": diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 669b6de7dd9..92ad5cfbcc2 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -131,6 +131,7 @@ public class GatewayBridge { private volatile DataSubscriberInfo execCmdSubInfo; private volatile DataSubscriberInfo shellCmdSubInfo; private volatile DataSubscriberInfo requestFilesFilenamesSubInfo; + private volatile DataSubscriberInfo requestFilesContentSubInfo; public GatewayBridge( SubscriptionService subscriptionService, @@ -208,6 +209,10 @@ public void init() { subscriptionService.registerCallback( EVENTS.requestFilesFilenames(), this::onRequestFilesFilenames); } + if (additionalIGEvents.contains(EVENTS.requestFilesContent())) { + subscriptionService.registerCallback( + EVENTS.requestFilesContent(), this::onRequestFilesContent); + } } /** @@ -235,6 +240,7 @@ public void reset() { execCmdSubInfo = null; shellCmdSubInfo = null; requestFilesFilenamesSubInfo = null; + requestFilesContentSubInfo = null; } private Flow onUser(final RequestContext ctx_, final String user) { @@ -605,6 +611,31 @@ private Flow onRequestFilesFilenames(RequestContext ctx_, List fil } } + private Flow onRequestFilesContent(RequestContext ctx_, List filesContent) { + AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); + if (ctx == null || filesContent == null || filesContent.isEmpty()) { + return NoopFlow.INSTANCE; + } + while (true) { + DataSubscriberInfo subInfo = requestFilesContentSubInfo; + if (subInfo == null) { + subInfo = producerService.getDataSubscribers(KnownAddresses.REQUEST_FILES_CONTENT); + requestFilesContentSubInfo = subInfo; + } + if (subInfo == null || subInfo.isEmpty()) { + return NoopFlow.INSTANCE; + } + DataBundle bundle = + new SingletonDataBundle<>(KnownAddresses.REQUEST_FILES_CONTENT, filesContent); + try { + GatewayContext gwCtx = new GatewayContext(false); + return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); + } catch (ExpiredSubscriberInfoException e) { + requestFilesContentSubInfo = null; + } + } + } + private Flow onDatabaseSqlQuery(RequestContext ctx_, String sql) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { @@ -1464,6 +1495,7 @@ private static class IGAppSecEventDependencies { DATA_DEPENDENCIES.put(KnownAddresses.REQUEST_BODY_OBJECT, l(EVENTS.requestBodyProcessed())); DATA_DEPENDENCIES.put( KnownAddresses.REQUEST_FILES_FILENAMES, l(EVENTS.requestFilesFilenames())); + DATA_DEPENDENCIES.put(KnownAddresses.REQUEST_FILES_CONTENT, l(EVENTS.requestFilesContent())); } private static Collection> l( diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecificationForkedTest.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecificationForkedTest.groovy index 06a1a61799d..9290d2726c1 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecificationForkedTest.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecificationForkedTest.groovy @@ -26,6 +26,7 @@ class KnownAddressesSpecificationForkedTest extends Specification { 'server.request.body.files_field_names', 'server.request.body.filenames', 'server.request.body.combined_file_size', + 'server.request.body.files_content', 'server.request.query', 'server.request.headers.no_cookies', 'grpc.server.method', @@ -58,7 +59,7 @@ class KnownAddressesSpecificationForkedTest extends Specification { void 'number of known addresses is expected number'() { expect: - Address.instanceCount() == 46 + Address.instanceCount() == 47 KnownAddresses.WAF_CONTEXT_PROCESSOR.serial == Address.instanceCount() - 1 } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeIGRegistrationSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeIGRegistrationSpecification.groovy index 7457fd23e98..617240790cf 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeIGRegistrationSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeIGRegistrationSpecification.groovy @@ -34,4 +34,15 @@ class GatewayBridgeIGRegistrationSpecification extends DDSpecification { then: 1 * ig.registerCallback(Events.REQUEST_BODY_DONE, _) } + + void 'requestFilesContent is registered via data address'() { + given: + 1 * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_FILES_CONTENT] + + when: + bridge.init() + + then: + 1 * ig.registerCallback(Events.get().requestFilesContent(), _) + } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 505c4950c22..10c7a4459b5 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -123,6 +123,7 @@ class GatewayBridgeSpecification extends DDSpecification { BiFunction> fileLoadedCB BiFunction> fileWrittenCB BiFunction, Flow> requestFilesFilenamesCB + BiFunction, Flow> requestFilesContentCB BiFunction> requestSessionCB BiFunction> execCmdCB BiFunction> shellCmdCB @@ -463,7 +464,7 @@ class GatewayBridgeSpecification extends DDSpecification { void callInitAndCaptureCBs() { // force all callbacks to be registered - _ * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_PATH_PARAMS, KnownAddresses.REQUEST_BODY_OBJECT, KnownAddresses.REQUEST_FILES_FILENAMES] + _ * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_PATH_PARAMS, KnownAddresses.REQUEST_BODY_OBJECT, KnownAddresses.REQUEST_FILES_FILENAMES, KnownAddresses.REQUEST_FILES_CONTENT] 1 * ig.registerCallback(EVENTS.requestStarted(), _) >> { requestStartedCB = it[1]; null @@ -561,6 +562,9 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * ig.registerCallback(EVENTS.requestFilesFilenames(), _) >> { requestFilesFilenamesCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.requestFilesContent(), _) >> { + requestFilesContentCB = it[1]; null + } 0 * ig.registerCallback(_, _) bridge.init() @@ -1142,6 +1146,38 @@ class GatewayBridgeSpecification extends DDSpecification { 0 * eventDispatcher.publishDataEvent(*_) } + void 'process request files content'() { + setup: + final filesContent = ['%PDF-1.4 malicious content', '#!/bin/bash\nrm -rf /'] + eventDispatcher.getDataSubscribers({ + KnownAddresses.REQUEST_FILES_CONTENT in it + }) >> nonEmptyDsInfo + DataBundle bundle + GatewayContext gatewayContext + + when: + Flow flow = requestFilesContentCB.apply(ctx, filesContent) + + then: + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { + a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE + } + bundle.get(KnownAddresses.REQUEST_FILES_CONTENT) == filesContent + flow.result == null + flow.action == Flow.Action.Noop.INSTANCE + gatewayContext.isTransient == false + gatewayContext.isRasp == false + } + + void 'process request files content with empty list returns noop'() { + when: + Flow flow = requestFilesContentCB.apply(ctx, []) + + then: + flow == NoopFlow.INSTANCE + 0 * eventDispatcher.publishDataEvent(*_) + } + void 'process exec cmd'() { setup: final cmd = ['/bin/../usr/bin/reboot', '-f'] as String[] diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index e9cfd59be33..a9b22a01f5b 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -17,6 +17,9 @@ 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.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.function.BiFunction; @@ -47,6 +50,9 @@ public void methodAdvice(MethodTransformer transformer) { @RequiresRequestContext(RequestContextSlot.APPSEC) public static class ParseRequestAdvice { + + static final int MAX_FILE_CONTENT_BYTES = 4096; + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( @Advice.Return final List fileItems, @@ -57,11 +63,6 @@ static void after( } CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - BiFunction, Flow> callback = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (callback == null) { - return; - } List filenames = new ArrayList<>(); for (FileItem fileItem : fileItems) { @@ -77,14 +78,65 @@ static void after( return; } - Flow flow = callback.apply(reqCtx, filenames); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + // Fire filenames event + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + if (filenamesCallback != null) { + Flow flow = filenamesCallback.apply(reqCtx, filenames); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + return; + } + } + } + + // Fire content event only if not blocked + BiFunction, Flow> contentCallback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (contentCallback == null) { + return; + } + List filesContent = new ArrayList<>(); + for (FileItem fileItem : fileItems) { + if (fileItem.isFormField()) { + continue; + } + String name = fileItem.getName(); + if (name == null || name.isEmpty()) { + continue; + } + String content = ""; + try { + InputStream is = fileItem.getInputStream(); + byte[] buf = new byte[MAX_FILE_CONTENT_BYTES]; + int total = 0; + int n; + while (total < MAX_FILE_CONTENT_BYTES + && (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) { + total += n; + } + content = new String(buf, 0, total, StandardCharsets.ISO_8859_1); + } catch (IOException ignored) { + } + filesContent.add(content); + } + if (filesContent.isEmpty()) { + return; + } + Flow contentFlow = contentCallback.apply(reqCtx, filesContent); + Flow.Action contentAction = contentFlow.getAction(); + if (contentAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction; BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (multipart file upload)"); + t = new BlockingException("Blocked request (multipart file upload content)"); reqCtx.getTraceSegment().effectivelyBlocked(); } } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index a9418b52c0a..90ba926be0a 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -230,6 +230,26 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { transformers: [], on_match : ['block'] ], + [ + id : '__test_file_upload_content_block', + name : 'test rule to block on malicious file upload content', + tags : [ + type : 'unrestricted-file-upload', + category : 'attack_attempt', + confidence: '1', + ], + conditions : [ + [ + parameters: [ + inputs: [[address: 'server.request.body.files_content']], + regex : 'dd-test-malicious-file-content', + ], + operator : 'match_regex', + ] + ], + transformers: [], + on_match : ['block'] + ], [ id : "apiA-100-001", name: "API 10 tag rule on request headers", @@ -611,6 +631,38 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } + void 'block request based on malicious file upload content'() { + when: + String url = "http://localhost:${httpPort}/upload" + def requestBody = new okhttp3.MultipartBody.Builder() + .setType(okhttp3.MultipartBody.FORM) + .addFormDataPart('file', 'safe-document.txt', + RequestBody.create(MediaType.parse('application/octet-stream'), 'dd-test-malicious-file-content')) + .build() + def request = new Request.Builder() + .url(url) + .post(requestBody) + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + responseBodyStr.contains("blocked") + response.code() == 403 + + when: + waitForTraceCount(1) == 1 + + then: + rootSpans.size() == 1 + forEachRootSpanTrigger { + assert it['rule']['id'] == '__test_file_upload_content_block' + } + rootSpans.each { + assert it.meta.get('appsec.blocked') != null, 'appsec.blocked is not set' + } + } + void 'rasp reports stacktrace on sql injection'() { when: String url = "http://localhost:${httpPort}/sqli/query?id=' OR 1=1 --" diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index d51261b1196..f2201f9d26d 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -396,7 +396,19 @@ public EventType, Flow>> requestFi REQUEST_FILES_FILENAMES; } - static final int FILE_WRITTEN_ID = 31; + static final int REQUEST_FILES_CONTENT_ID = 31; + + @SuppressWarnings("rawtypes") + private static final EventType REQUEST_FILES_CONTENT = + new ET<>("request.body.files.content", REQUEST_FILES_CONTENT_ID); + + /** Contents of files uploaded in a multipart/form-data request */ + @SuppressWarnings("unchecked") + public EventType, Flow>> requestFilesContent() { + return (EventType, Flow>>) REQUEST_FILES_CONTENT; + } + + static final int FILE_WRITTEN_ID = 32; @SuppressWarnings("rawtypes") private static final EventType FILE_WRITTEN = new ET<>("file.written", FILE_WRITTEN_ID); diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 0e8a4174117..0de586969a4 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -19,6 +19,7 @@ import static datadog.trace.api.gateway.Events.REQUEST_BODY_START_ID; import static datadog.trace.api.gateway.Events.REQUEST_CLIENT_SOCKET_ADDRESS_ID; import static datadog.trace.api.gateway.Events.REQUEST_ENDED_ID; +import static datadog.trace.api.gateway.Events.REQUEST_FILES_CONTENT_ID; import static datadog.trace.api.gateway.Events.REQUEST_FILES_FILENAMES_ID; import static datadog.trace.api.gateway.Events.REQUEST_HEADER_DONE_ID; import static datadog.trace.api.gateway.Events.REQUEST_HEADER_ID; @@ -363,6 +364,7 @@ public Flow apply(RequestContext ctx, StoredBodySupplier storedBodySupplie case REQUEST_BODY_CONVERTED_ID: case RESPONSE_BODY_ID: case REQUEST_FILES_FILENAMES_ID: + case REQUEST_FILES_CONTENT_ID: return (C) new BiFunction>() { @Override From 0817b8f2d0ba6a85f26d7c74d042bf1a9c979baa Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 17 Apr 2026 09:20:54 +0200 Subject: [PATCH 2/7] refactor(appsec): fetch both callbacks upfront before iterating file items --- .../fileupload/CommonsFileUploadAppSecModule.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index a9b22a01f5b..0f87bd0db8e 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -63,6 +63,13 @@ static void after( } CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCallback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (filenamesCallback == null && contentCallback == null) { + return; + } List filenames = new ArrayList<>(); for (FileItem fileItem : fileItems) { @@ -79,8 +86,6 @@ static void after( } // Fire filenames event - BiFunction, Flow> filenamesCallback = - cbp.getCallback(EVENTS.requestFilesFilenames()); if (filenamesCallback != null) { Flow flow = filenamesCallback.apply(reqCtx, filenames); Flow.Action action = flow.getAction(); @@ -97,8 +102,6 @@ static void after( } // Fire content event only if not blocked - BiFunction, Flow> contentCallback = - cbp.getCallback(EVENTS.requestFilesContent()); if (contentCallback == null) { return; } From dfe31cead163753ffedc4cfb7afdc8cb97f114a5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 17 Apr 2026 09:36:35 +0200 Subject: [PATCH 3/7] test(appsec): add unit tests for readContent truncation in CommonsFileUploadAppSecModule --- .../CommonsFileUploadAppSecModule.java | 31 +++++---- .../CommonsFileUploadAppSecModuleTest.groovy | 64 +++++++++++++++++++ 2 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index 0f87bd0db8e..f66536063ce 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -114,20 +114,7 @@ static void after( if (name == null || name.isEmpty()) { continue; } - String content = ""; - try { - InputStream is = fileItem.getInputStream(); - byte[] buf = new byte[MAX_FILE_CONTENT_BYTES]; - int total = 0; - int n; - while (total < MAX_FILE_CONTENT_BYTES - && (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) { - total += n; - } - content = new String(buf, 0, total, StandardCharsets.ISO_8859_1); - } catch (IOException ignored) { - } - filesContent.add(content); + filesContent.add(readContent(fileItem)); } if (filesContent.isEmpty()) { return; @@ -144,5 +131,21 @@ static void after( } } } + + static String readContent(FileItem fileItem) { + try { + InputStream is = fileItem.getInputStream(); + byte[] buf = new byte[MAX_FILE_CONTENT_BYTES]; + int total = 0; + int n; + while (total < MAX_FILE_CONTENT_BYTES + && (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) { + total += n; + } + return new String(buf, 0, total, StandardCharsets.ISO_8859_1); + } catch (IOException ignored) { + return ""; + } + } } } diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy new file mode 100644 index 00000000000..36a434d39fa --- /dev/null +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy @@ -0,0 +1,64 @@ +import datadog.trace.instrumentation.commons.fileupload.CommonsFileUploadAppSecModule +import org.apache.commons.fileupload.FileItem +import spock.lang.Specification + +class CommonsFileUploadAppSecModuleTest extends Specification { + + def "readContent returns full content when smaller than limit"() { + given: + def content = 'Hello, World!' + def item = fileItem(content) + + expect: + CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == content + } + + def "readContent truncates content to MAX_FILE_CONTENT_BYTES"() { + given: + def largeContent = 'X' * (CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + 500) + def item = fileItem(largeContent) + + when: + def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) + + then: + result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + result == 'X' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + } + + def "readContent returns empty string when getInputStream throws"() { + given: + FileItem item = Stub(FileItem) + item.getInputStream() >> { throw new IOException('simulated error') } + + expect: + CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == '' + } + + def "readContent returns empty string for empty content"() { + given: + def item = fileItem('') + + expect: + CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == '' + } + + def "readContent reads exactly MAX_FILE_CONTENT_BYTES when content equals the limit"() { + given: + def content = 'A' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + def item = fileItem(content) + + when: + def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) + + then: + result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + result == content + } + + private FileItem fileItem(String content) { + FileItem item = Stub(FileItem) + item.getInputStream() >> new ByteArrayInputStream(content.getBytes('ISO-8859-1')) + return item + } +} From 304846b64d8c87d8c8b1808900fc443782d6700e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 17 Apr 2026 09:57:48 +0200 Subject: [PATCH 4/7] fix(appsec): close file item stream after reading content to avoid fd leak --- .../commons/fileupload/CommonsFileUploadAppSecModule.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index f66536063ce..829488c848e 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -133,8 +133,7 @@ static void after( } static String readContent(FileItem fileItem) { - try { - InputStream is = fileItem.getInputStream(); + try (InputStream is = fileItem.getInputStream()) { byte[] buf = new byte[MAX_FILE_CONTENT_BYTES]; int total = 0; int n; From 22d70a07f45a69a71ebd3641afef4d9df5385d21 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 17 Apr 2026 14:55:01 +0200 Subject: [PATCH 5/7] fix(test): register requestFilesContent in InstrumentationGatewayTest event coverage --- .../trace/api/gateway/InstrumentationGatewayTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index c23331f7341..3ae4aba9a71 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -240,6 +240,10 @@ public void testNormalCalls() { assertEquals( Flow.Action.Noop.INSTANCE, cbp.getCallback(events.requestFilesFilenames()).apply(null, null).getAction()); + ss.registerCallback(events.requestFilesContent(), callback); + assertEquals( + Flow.Action.Noop.INSTANCE, + cbp.getCallback(events.requestFilesContent()).apply(null, null).getAction()); ss.registerCallback(events.fileWritten(), callback); cbp.getCallback(events.fileWritten()).apply(null, null); assertEquals(Events.MAX_EVENTS, callback.count); @@ -331,6 +335,9 @@ public void testThrowableBlocking() { ss.registerCallback(events.requestFilesFilenames(), throwback); assertEquals( Flow.ResultFlow.empty(), cbp.getCallback(events.requestFilesFilenames()).apply(null, null)); + ss.registerCallback(events.requestFilesContent(), throwback); + assertEquals( + Flow.ResultFlow.empty(), cbp.getCallback(events.requestFilesContent()).apply(null, null)); ss.registerCallback(events.fileWritten(), throwback); cbp.getCallback(events.fileWritten()).apply(null, null); assertEquals(Events.MAX_EVENTS, throwback.count); From 2076c7b8581159619397f296529754b732bf1306 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 17 Apr 2026 16:19:56 +0200 Subject: [PATCH 6/7] fix(appsec): extract readContent to helper class to fix muzzle failure The static readContent method in ParseRequestAdvice created a self-reference in the inlined advice bytecode (invokestatic on CommonsFileUploadAppSecModule$ParseRequestAdvice) that muzzle could not resolve in the application classloader, causing the instrumentation to be silently skipped. Moves readContent to a new FileItemContentReader helper class declared via helperClassNames(), which muzzle skips and the HelperInjector injects into the application classloader at runtime. --- .../CommonsFileUploadAppSecModule.java | 29 +++++-------------- .../fileupload/FileItemContentReader.java | 28 ++++++++++++++++++ .../CommonsFileUploadAppSecModuleTest.groovy | 26 ++++++++--------- 3 files changed, 49 insertions(+), 34 deletions(-) create mode 100644 dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index 829488c848e..2436de8e51a 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -17,9 +17,6 @@ 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.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.function.BiFunction; @@ -39,6 +36,13 @@ public String instrumentedType() { return "org.apache.commons.fileupload.servlet.ServletFileUpload"; } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.commons.fileupload.FileItemContentReader", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -51,8 +55,6 @@ public void methodAdvice(MethodTransformer transformer) { @RequiresRequestContext(RequestContextSlot.APPSEC) public static class ParseRequestAdvice { - static final int MAX_FILE_CONTENT_BYTES = 4096; - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after( @Advice.Return final List fileItems, @@ -114,7 +116,7 @@ static void after( if (name == null || name.isEmpty()) { continue; } - filesContent.add(readContent(fileItem)); + filesContent.add(FileItemContentReader.readContent(fileItem)); } if (filesContent.isEmpty()) { return; @@ -131,20 +133,5 @@ static void after( } } } - - static String readContent(FileItem fileItem) { - try (InputStream is = fileItem.getInputStream()) { - byte[] buf = new byte[MAX_FILE_CONTENT_BYTES]; - int total = 0; - int n; - while (total < MAX_FILE_CONTENT_BYTES - && (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) { - total += n; - } - return new String(buf, 0, total, StandardCharsets.ISO_8859_1); - } catch (IOException ignored) { - return ""; - } - } } } diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java new file mode 100644 index 00000000000..055c235f03f --- /dev/null +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java @@ -0,0 +1,28 @@ +package datadog.trace.instrumentation.commons.fileupload; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import org.apache.commons.fileupload.FileItem; + +/** Helper class injected into the application classloader by the AppSec instrumentation. */ +public final class FileItemContentReader { + public static final int MAX_CONTENT_BYTES = 4096; + + public static String readContent(FileItem fileItem) { + try (InputStream is = fileItem.getInputStream()) { + byte[] buf = new byte[MAX_CONTENT_BYTES]; + int total = 0; + int n; + while (total < MAX_CONTENT_BYTES + && (n = is.read(buf, total, MAX_CONTENT_BYTES - total)) != -1) { + total += n; + } + return new String(buf, 0, total, StandardCharsets.ISO_8859_1); + } catch (IOException ignored) { + return ""; + } + } + + private FileItemContentReader() {} +} diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy index 36a434d39fa..38d1bcef8f1 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/CommonsFileUploadAppSecModuleTest.groovy @@ -1,4 +1,4 @@ -import datadog.trace.instrumentation.commons.fileupload.CommonsFileUploadAppSecModule +import datadog.trace.instrumentation.commons.fileupload.FileItemContentReader import org.apache.commons.fileupload.FileItem import spock.lang.Specification @@ -10,20 +10,20 @@ class CommonsFileUploadAppSecModuleTest extends Specification { def item = fileItem(content) expect: - CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == content + FileItemContentReader.readContent(item) == content } - def "readContent truncates content to MAX_FILE_CONTENT_BYTES"() { + def "readContent truncates content to MAX_CONTENT_BYTES"() { given: - def largeContent = 'X' * (CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + 500) + def largeContent = 'X' * (FileItemContentReader.MAX_CONTENT_BYTES + 500) def item = fileItem(largeContent) when: - def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) + def result = FileItemContentReader.readContent(item) then: - result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES - result == 'X' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + result.length() == FileItemContentReader.MAX_CONTENT_BYTES + result == 'X' * FileItemContentReader.MAX_CONTENT_BYTES } def "readContent returns empty string when getInputStream throws"() { @@ -32,7 +32,7 @@ class CommonsFileUploadAppSecModuleTest extends Specification { item.getInputStream() >> { throw new IOException('simulated error') } expect: - CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == '' + FileItemContentReader.readContent(item) == '' } def "readContent returns empty string for empty content"() { @@ -40,19 +40,19 @@ class CommonsFileUploadAppSecModuleTest extends Specification { def item = fileItem('') expect: - CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) == '' + FileItemContentReader.readContent(item) == '' } - def "readContent reads exactly MAX_FILE_CONTENT_BYTES when content equals the limit"() { + def "readContent reads exactly MAX_CONTENT_BYTES when content equals the limit"() { given: - def content = 'A' * CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + def content = 'A' * FileItemContentReader.MAX_CONTENT_BYTES def item = fileItem(content) when: - def result = CommonsFileUploadAppSecModule.ParseRequestAdvice.readContent(item) + def result = FileItemContentReader.readContent(item) then: - result.length() == CommonsFileUploadAppSecModule.ParseRequestAdvice.MAX_FILE_CONTENT_BYTES + result.length() == FileItemContentReader.MAX_CONTENT_BYTES result == content } From f76f3891e971b0224af8a404557f701e17aee67c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 20 Apr 2026 13:01:24 +0200 Subject: [PATCH 7/7] fix(appsec): cap number of file contents sent to WAF at 25 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without a bound, uploading N files would pass up to N × 4096 bytes to the WAF in a single call. MAX_FILES_TO_INSPECT = 25 limits total content to at most 100 KB, consistent with the per-file MAX_CONTENT_BYTES cap. --- .../commons/fileupload/CommonsFileUploadAppSecModule.java | 3 +++ .../commons/fileupload/FileItemContentReader.java | 1 + 2 files changed, 4 insertions(+) diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java index 2436de8e51a..f64922a6511 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java @@ -109,6 +109,9 @@ static void after( } List filesContent = new ArrayList<>(); for (FileItem fileItem : fileItems) { + if (filesContent.size() >= FileItemContentReader.MAX_FILES_TO_INSPECT) { + break; + } if (fileItem.isFormField()) { continue; } diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java index 055c235f03f..5a0f85eb310 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/FileItemContentReader.java @@ -8,6 +8,7 @@ /** Helper class injected into the application classloader by the AppSec instrumentation. */ public final class FileItemContentReader { public static final int MAX_CONTENT_BYTES = 4096; + public static final int MAX_FILES_TO_INSPECT = 25; public static String readContent(FileItem fileItem) { try (InputStream is = fileItem.getInputStream()) {