diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index ac7e2fd17b7..92f07ca6a8f 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -136,6 +136,7 @@ abstract class HttpServerTest extends WithHttpServer { ss.registerCallback(events.requestBodyDone(), callbacks.requestBodyEndCb) ss.registerCallback(events.requestBodyProcessed(), callbacks.requestBodyObjectCb) ss.registerCallback(events.requestFilesFilenames(), callbacks.requestFilesFilenamesCb) + ss.registerCallback(events.requestFilesContent(), callbacks.requestFilesContentCb) ss.registerCallback(events.responseBody(), callbacks.responseBodyObjectCb) ss.registerCallback(events.responseStarted(), callbacks.responseStartedCb) ss.registerCallback(events.responseHeader(), callbacks.responseHeaderCb) @@ -372,6 +373,10 @@ abstract class HttpServerTest extends WithHttpServer { false } + boolean testBodyFilesContent() { + false + } + boolean testBodyJson() { false } @@ -1652,6 +1657,82 @@ abstract class HttpServerTest extends WithHttpServer { response.close() } + def 'test instrumentation gateway file upload content'() { + setup: + assumeTrue(testBodyFilesContent()) + RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content') + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', 'test.bin', fileBody) + .build() + def httpRequest = request(BODY_MULTIPART, 'POST', body).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + it.getTag('request.body.files_content') == '[file content]' + } + + cleanup: + response.close() + } + + def 'test instrumentation gateway file upload content truncated at max size'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxContentBytes = Config.get().getAppSecMaxFileContentBytes() + def body = new MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart('file', 'large.bin', + RequestBody.create(MediaType.parse('application/octet-stream'), 'X' * (maxContentBytes + 500))) + .build() + def httpRequest = request(BODY_MULTIPART, 'POST', body).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + span -> + span.getTag('request.body.files_content') == '[' + 'X' * maxContentBytes + ']' + } + + cleanup: + response.close() + } + + def 'test instrumentation gateway file upload content max files limit'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() + def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) + (1..maxFilesToInspect + 1).each { + i -> + bodyBuilder.addFormDataPart("file$i", "file${i}.bin", + RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_$i")) + } + def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + span -> + def tag = span.getTag('request.body.files_content') as String + tag?.contains("content_of_file_$maxFilesToInspect") && + !tag.contains("content_of_file_${maxFilesToInspect + 1}") + } + + cleanup: + response.close() + } + def 'test instrumentation gateway json request body'() { setup: assumeTrue(testBodyJson()) @@ -2589,6 +2670,7 @@ abstract class HttpServerTest extends WithHttpServer { boolean responseBodyTag Object responseBody List uploadedFilenames + List uploadedFilesContent } static final String stringOrEmpty(String string) { @@ -2765,6 +2847,15 @@ abstract class HttpServerTest extends WithHttpServer { Flow.ResultFlow.empty() } as BiFunction, Flow>) + final BiFunction, Flow> requestFilesContentCb = + ({ + RequestContext rqCtxt, List contents -> + rqCtxt.traceSegment.setTagTop('request.body.files_content', contents as String) + Context context = rqCtxt.getData(RequestContextSlot.APPSEC) + context.uploadedFilesContent = contents + Flow.ResultFlow.empty() + } as BiFunction, Flow>) + final BiFunction> responseBodyObjectCb = ({ RequestContext rqCtxt, Object obj -> diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecInstrumentation.java b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecInstrumentation.java index 8d2b12f7e95..5ad79ee2e1e 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecInstrumentation.java +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecInstrumentation.java @@ -73,22 +73,23 @@ static void after( return; } - List filenames = new ArrayList<>(); + List filenames = filenamesCallback != null ? new ArrayList<>() : null; + List filesContent = contentCallback != null ? new ArrayList<>() : null; for (FileItem fileItem : fileItems) { if (fileItem.isFormField()) { continue; } String name = fileItem.getName(); - if (name != null && !name.isEmpty()) { + if (filenames != null && name != null && !name.isEmpty()) { filenames.add(name); } - } - if (filenames.isEmpty() && contentCallback == null) { - return; + if (filesContent != null + && filesContent.size() < FileItemContentReader.MAX_FILES_TO_INSPECT) { + filesContent.add(FileItemContentReader.readContent(fileItem)); + } } - // Fire filenames event - if (filenamesCallback != null && !filenames.isEmpty()) { + if (filenames != null && !filenames.isEmpty()) { Flow flow = filenamesCallback.apply(reqCtx, filenames); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { @@ -98,28 +99,21 @@ static void after( brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); t = new BlockingException("Blocked request (multipart file upload)"); reqCtx.getTraceSegment().effectivelyBlocked(); - return; } } } - // Fire content event only if not blocked - if (contentCallback == null) { - return; - } - List filesContent = FileItemContentReader.readContents(fileItems); - 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 content)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + if (t == null && filesContent != null && !filesContent.isEmpty()) { + 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 content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } 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 1686f66de61..9940c5a78d3 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 @@ -1,41 +1,20 @@ package datadog.trace.instrumentation.commons.fileupload; +import datadog.trace.api.Config; import datadog.trace.api.http.MultipartContentDecoder; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; -import java.util.List; import org.apache.commons.fileupload.FileItem; /** Reads uploaded file content for WAF inspection. */ public final class FileItemContentReader { - public static final int MAX_CONTENT_BYTES = 4096; - public static final int MAX_FILES_TO_INSPECT = 25; - - public static List readContents(List fileItems) { - List result = new ArrayList<>(); - for (FileItem fileItem : fileItems) { - if (result.size() >= MAX_FILES_TO_INSPECT) { - break; - } - if (fileItem.isFormField()) { - continue; - } - result.add(readContent(fileItem)); - } - return result; - } + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); 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 MultipartContentDecoder.decodeBytes(buf, total, fileItem.getContentType()); + return MultipartContentDecoder.readInputStream( + is, MAX_CONTENT_BYTES, fileItem.getContentType()); } catch (IOException ignored) { return ""; } diff --git a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/FileItemContentReaderTest.groovy b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/FileItemContentReaderTest.groovy index 4940c46bcbf..25968b4217b 100644 --- a/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/FileItemContentReaderTest.groovy +++ b/dd-java-agent/instrumentation/commons-fileupload-1.5/src/test/groovy/FileItemContentReaderTest.groovy @@ -48,63 +48,6 @@ class FileItemContentReaderTest extends Specification { FileItemContentReader.readContent(item) == text } - void 'readContents returns content for each non-form file with a name'() { - given: - def items = [fileItem('content-a', 'file-a.txt'), fileItem('content-b', 'file-b.txt'),] - - when: - def result = FileItemContentReader.readContents(items) - - then: - result == ['content-a', 'content-b'] - } - - void 'readContents skips form fields'() { - given: - FileItem formField = Stub(FileItem) - formField.isFormField() >> true - def items = [formField, fileItem('content', 'real.txt')] - - when: - def result = FileItemContentReader.readContents(items) - - then: - result == ['content'] - } - - void 'readContents includes file parts with empty or null name'() { - given: - def items = [ - fileItem('content-no-name', null), - fileItem('content-empty-name', ''), - fileItem('content-named', 'named.txt'), - ] - - when: - def result = FileItemContentReader.readContents(items) - - then: - result == ['content-no-name', 'content-empty-name', 'content-named'] - } - - void 'readContents stops after MAX_FILES_TO_INSPECT files'() { - given: - def items = (1..FileItemContentReader.MAX_FILES_TO_INSPECT + 1).collect { - fileItem("content-${it}", "file-${it}.txt") - } - - when: - def result = FileItemContentReader.readContents(items) - - then: - result.size() == FileItemContentReader.MAX_FILES_TO_INSPECT - } - - void 'readContents returns empty list for empty input'() { - expect: - FileItemContentReader.readContents([]) == [] - } - private FileItem fileItem(String content) { fileItem(content, 'file.txt') } diff --git a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/test/groovy/GrizzlyTest.groovy b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/test/groovy/GrizzlyTest.groovy index b10c86f6fff..18b4ce8aec4 100644 --- a/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/test/groovy/GrizzlyTest.groovy +++ b/dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/src/test/groovy/GrizzlyTest.groovy @@ -49,6 +49,11 @@ class GrizzlyTest extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy index 3fa5a4672ad..b2d8dd20751 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy +++ b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy @@ -54,6 +54,11 @@ class Jersey2JettyTest extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/Jersey3JettyTest.groovy b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/Jersey3JettyTest.groovy index b8076813660..a0a94ec3136 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/Jersey3JettyTest.groovy +++ b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/Jersey3JettyTest.groovy @@ -53,6 +53,11 @@ class Jersey3JettyTest extends HttpServerTest { true } + @Override + boolean testBodyFilenames() { + true + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/build.gradle b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/build.gradle index 2d6dc729a5b..4bd297d5456 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/build.gradle +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/build.gradle @@ -31,10 +31,16 @@ muzzle { apply from: "$rootDir/gradle/java.gradle" +configurations.configureEach { + resolutionStrategy.deactivateDependencyLocking() +} + dependencies { compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '2.0' compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '2.0' compileOnly group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '2.0' + + testImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '2.18' } // tested in grizzly-http-2.3.20 diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartHelper.java b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartHelper.java new file mode 100644 index 00000000000..263aca69722 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartHelper.java @@ -0,0 +1,85 @@ +package datadog.trace.instrumentation.jersey2; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +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.util.ArrayList; +import java.util.List; +import java.util.Map; +import javax.ws.rs.core.MediaType; +import org.glassfish.jersey.media.multipart.FormDataBodyPart; +import org.glassfish.jersey.media.multipart.FormDataContentDisposition; +import org.glassfish.jersey.message.internal.MediaTypes; + +public final class MultiPartHelper { + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + private MultiPartHelper() {} + + public static void collectBodyPart( + FormDataBodyPart bodyPart, + Map> bodyMap, + List filenames, + List filesContent) { + if (bodyMap != null + && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) { + // BodyPartEntity allows re-reading the part without consuming the stream + bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue()); + } + FormDataContentDisposition cd; + try { + cd = bodyPart.getFormDataContentDisposition(); + } catch (Exception ignored) { + // IllegalArgumentException on malformed Content-Disposition: skip this part gracefully + // so a single bad part does not abort processing of the remaining parts. + cd = null; + } + // rawFilename == null → no filename attribute → form field → skip filenames and content + // rawFilename == "" → filename attribute present but empty → content YES, filenames NO + // rawFilename != "" → file with name → both + String rawFilename = cd != null ? cd.getFileName() : null; + if (filenames != null && rawFilename != null && !rawFilename.isEmpty()) { + filenames.add(rawFilename); + } + if (filesContent != null && rawFilename != null && filesContent.size() < MAX_FILES_TO_INSPECT) { + filesContent.add(readContent(bodyPart)); + } + } + + public static String readContent(FormDataBodyPart bodyPart) { + try { + // getEntityAs(InputStream.class) is backed by BodyPartEntity which supports re-reading: + // each call creates a fresh stream from the buffered MIME part data. + try (InputStream is = bodyPart.getEntityAs(InputStream.class)) { + if (is == null) return ""; + String contentType = + bodyPart.getMediaType() != null ? bodyPart.getMediaType().toString() : null; + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } catch (IOException ignored) { + return ""; + } + } + + public static BlockingException tryBlock(RequestContext ctx, Flow flow, String message) { + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + BlockingException be = new BlockingException(message); + ctx.getTraceSegment().effectivelyBlocked(); + return be; + } + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java index 16431c4f01c..bd827c53819 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/main/java/datadog/trace/instrumentation/jersey2/MultiPartReaderServerSideInstrumentation.java @@ -12,7 +12,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -23,12 +22,10 @@ import java.util.List; import java.util.Map; import java.util.function.BiFunction; -import javax.ws.rs.core.MediaType; import net.bytebuddy.asm.Advice; import org.glassfish.jersey.media.multipart.BodyPart; import org.glassfish.jersey.media.multipart.FormDataBodyPart; import org.glassfish.jersey.media.multipart.MultiPart; -import org.glassfish.jersey.message.internal.MediaTypes; @AutoService(InstrumenterModule.class) public class MultiPartReaderServerSideInstrumentation extends InstrumenterModule.AppSec @@ -48,6 +45,11 @@ public String instrumentedType() { return "org.glassfish.jersey.media.multipart.internal.MultiPartReaderServerSide"; } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".MultiPartHelper"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -72,42 +74,55 @@ static void after( CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); BiFunction> callback = cbp.getCallback(EVENTS.requestBodyProcessed()); - if (callback == null) { + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCallback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (callback == null && filenamesCallback == null && contentCallback == null) { return; } - Map> map = new HashMap<>(); + Map> map = callback != null ? new HashMap<>() : null; + List filenames = filenamesCallback != null ? new ArrayList<>() : null; + List filesContent = contentCallback != null ? new ArrayList<>() : null; for (BodyPart bodyPart : ret.getBodyParts()) { if (!(bodyPart instanceof FormDataBodyPart)) { continue; } - FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart; - if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) { - continue; - } - // if the type of dataBodyPart.getEntity() is BodyPartEntity, it is safe to read the part - // more than once. So we're not depriving the application of the data by consuming it here - String v = dataBodyPart.getValue(); + MultiPartHelper.collectBodyPart((FormDataBodyPart) bodyPart, map, filenames, filesContent); + } - String name = dataBodyPart.getName(); - List values = map.get(name); - if (values == null) { - values = new ArrayList<>(); - map.put(name, values); + if (map != null) { + Flow flow = callback.apply(reqCtx, map); + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, flow, "Blocked request (for MultiPartReaderServerSide/readMultiPart)"); + if (be != null) { + t = be; } + } - values.add(v); + if (filenames != null && !filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + if (t == null) { + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, filenamesFlow, "Blocked request (multipart file upload)"); + if (be != null) { + t = be; + } + } } - Flow flow = callback.apply(reqCtx, map); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + if (filesContent != null && !filesContent.isEmpty()) { + Flow contentFlow = contentCallback.apply(reqCtx, filesContent); + if (t == null) { + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, contentFlow, "Blocked request (multipart file upload content)"); + if (be != null) { + t = be; + } } } } diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/test/groovy/MultiPartHelperTest.groovy b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/test/groovy/MultiPartHelperTest.groovy new file mode 100644 index 00000000000..56287e7c4f1 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/test/groovy/MultiPartHelperTest.groovy @@ -0,0 +1,280 @@ +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.api.gateway.BlockResponseFunction +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.internal.TraceSegment +import datadog.trace.instrumentation.jersey2.MultiPartHelper +import org.glassfish.jersey.media.multipart.FormDataBodyPart +import org.glassfish.jersey.media.multipart.FormDataContentDisposition +import spock.lang.Specification + +import javax.ws.rs.core.MediaType + +class MultiPartHelperTest extends Specification { + + // collectBodyPart — body map + + def "text/plain part is added to body map"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'field' + bodyPart.getValue() >> 'value' + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map == [field: ['value']] + } + + def "non-text/plain part is not added to body map"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map.isEmpty() + } + + def "null body map is skipped without error"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getFormDataContentDisposition() >> null + + expect: + MultiPartHelper.collectBodyPart(bodyPart, null, null, null) + } + + def "multiple values for same field are accumulated"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'tag' + bodyPart.getValue() >>> ['a', 'b'] + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map == [tag: ['a', 'b']] + } + + // collectBodyPart — filenames + + def "filename is added to list when present"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + def filenames = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, null) + + then: + filenames == ['report.php'] + } + + def "null filenames list is skipped without error"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'f' + bodyPart.getValue() >> 'v' + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.collectBodyPart(bodyPart, [:], null, null) + } + + def "text/plain part with filename populates both body map and filename list"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'upload.txt' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'file' + bodyPart.getValue() >> 'content' + bodyPart.getFormDataContentDisposition() >> cd + def map = [:] + def filenames = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, filenames, null) + + then: + map == [file: ['content']] + filenames == ['upload.txt'] + } + + def "empty filename adds to content but not filenames"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> '' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('data'.bytes) + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content == ['data'] + } + + def "null filename (no filename attr) skips both filenames and content"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> null + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content.isEmpty() + } + + def "non-empty filename adds to both filenames and content"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> { throw new IllegalArgumentException("bad CD") } + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content.isEmpty() + noExceptionThrown() + } + + def "MAX_FILES_TO_INSPECT limits number of content entries"() { + given: + def parts = (1..MultiPartHelper.MAX_FILES_TO_INSPECT + 2).collect { i -> + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> "f${i}.bin" + def bp = Mock(FormDataBodyPart) + bp.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bp.getFormDataContentDisposition() >> cd + bp.getEntityAs(InputStream) >> new ByteArrayInputStream("content${i}".bytes) + bp + } + def content = [] + + when: + parts.each { MultiPartHelper.collectBodyPart(it, null, null, content) } + + then: + content.size() == MultiPartHelper.MAX_FILES_TO_INSPECT + } + + // tryBlock + + def "tryBlock returns null when flow action is not a blocking action"() { + given: + Flow flow = Stub(Flow) + flow.getAction() >> Flow.Action.Noop.INSTANCE + RequestContext ctx = Stub(RequestContext) + + expect: + MultiPartHelper.tryBlock(ctx, flow, 'msg') == null + } + + def "tryBlock returns BlockingException with provided message when brf commits response"() { + given: + def segment = Stub(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Stub(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + def result = MultiPartHelper.tryBlock(ctx, flow, 'blocked!') + + then: + result instanceof BlockingException + result.message == 'blocked!' + } + + def "tryBlock calls tryCommitBlockingResponse and effectivelyBlocked"() { + given: + def segment = Mock(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Mock(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + MultiPartHelper.tryBlock(ctx, flow, 'msg') + + then: + 1 * brf.tryCommitBlockingResponse(segment, rba) + 1 * segment.effectivelyBlocked() + } + + def "tryBlock returns null when brf is null despite blocking action"() { + given: + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> null + + expect: + MultiPartHelper.tryBlock(ctx, flow, 'msg') == null + } +} diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/build.gradle b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/build.gradle index f63738ca7c6..8c4054d4391 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/build.gradle +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/build.gradle @@ -24,10 +24,20 @@ muzzle { apply from: "$rootDir/gradle/java.gradle" +testJvmConstraints { + minJavaVersion = JavaVersion.VERSION_11 +} + +configurations.configureEach { + resolutionStrategy.deactivateDependencyLocking() +} + dependencies { compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '3.0.0' compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '3.0.0' compileOnly group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.0.0' + + testImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: '3.1.2' } // tested in GrizzlyTest/GrizzlyAsyncTest diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartHelper.java b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartHelper.java new file mode 100644 index 00000000000..0792c4a395b --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartHelper.java @@ -0,0 +1,85 @@ +package datadog.trace.instrumentation.jersey3; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.http.MultipartContentDecoder; +import jakarta.ws.rs.core.MediaType; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.glassfish.jersey.media.multipart.FormDataBodyPart; +import org.glassfish.jersey.media.multipart.FormDataContentDisposition; +import org.glassfish.jersey.message.internal.MediaTypes; + +public final class MultiPartHelper { + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + private MultiPartHelper() {} + + public static void collectBodyPart( + FormDataBodyPart bodyPart, + Map> bodyMap, + List filenames, + List filesContent) { + if (bodyMap != null + && MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, bodyPart.getMediaType())) { + // BodyPartEntity allows re-reading the part without consuming the stream + bodyMap.computeIfAbsent(bodyPart.getName(), k -> new ArrayList<>()).add(bodyPart.getValue()); + } + FormDataContentDisposition cd; + try { + cd = bodyPart.getFormDataContentDisposition(); + } catch (Exception ignored) { + // IllegalArgumentException on malformed Content-Disposition: skip this part gracefully + // so a single bad part does not abort processing of the remaining parts. + cd = null; + } + // rawFilename == null → no filename attribute → form field → skip filenames and content + // rawFilename == "" → filename attribute present but empty → content YES, filenames NO + // rawFilename != "" → file with name → both + String rawFilename = cd != null ? cd.getFileName() : null; + if (filenames != null && rawFilename != null && !rawFilename.isEmpty()) { + filenames.add(rawFilename); + } + if (filesContent != null && rawFilename != null && filesContent.size() < MAX_FILES_TO_INSPECT) { + filesContent.add(readContent(bodyPart)); + } + } + + public static String readContent(FormDataBodyPart bodyPart) { + try { + // getEntityAs(InputStream.class) is backed by BodyPartEntity which supports re-reading: + // each call creates a fresh stream from the buffered MIME part data. + try (InputStream is = bodyPart.getEntityAs(InputStream.class)) { + if (is == null) return ""; + String contentType = + bodyPart.getMediaType() != null ? bodyPart.getMediaType().toString() : null; + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } + } catch (IOException ignored) { + return ""; + } + } + + public static BlockingException tryBlock(RequestContext ctx, Flow flow, String message) { + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + BlockingException be = new BlockingException(message); + ctx.getTraceSegment().effectivelyBlocked(); + return be; + } + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartReaderServerSideInstrumentation.java b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartReaderServerSideInstrumentation.java index e09f053cf9e..4f441835b50 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartReaderServerSideInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/main/java/datadog/trace/instrumentation/jersey3/MultiPartReaderServerSideInstrumentation.java @@ -12,13 +12,11 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import jakarta.ws.rs.core.MediaType; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -28,7 +26,6 @@ import org.glassfish.jersey.media.multipart.BodyPart; import org.glassfish.jersey.media.multipart.FormDataBodyPart; import org.glassfish.jersey.media.multipart.MultiPart; -import org.glassfish.jersey.message.internal.MediaTypes; @AutoService(InstrumenterModule.class) public class MultiPartReaderServerSideInstrumentation extends InstrumenterModule.AppSec @@ -48,6 +45,11 @@ public String instrumentedType() { return "org.glassfish.jersey.media.multipart.internal.MultiPartReaderServerSide"; } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".MultiPartHelper"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -72,42 +74,55 @@ static void after( CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); BiFunction> callback = cbp.getCallback(EVENTS.requestBodyProcessed()); - if (callback == null) { + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCallback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (callback == null && filenamesCallback == null && contentCallback == null) { return; } - Map> map = new HashMap<>(); + Map> map = callback != null ? new HashMap<>() : null; + List filenames = filenamesCallback != null ? new ArrayList<>() : null; + List filesContent = contentCallback != null ? new ArrayList<>() : null; for (BodyPart bodyPart : ret.getBodyParts()) { if (!(bodyPart instanceof FormDataBodyPart)) { continue; } - FormDataBodyPart dataBodyPart = (FormDataBodyPart) bodyPart; - if (!MediaTypes.typeEqual(MediaType.TEXT_PLAIN_TYPE, dataBodyPart.getMediaType())) { - continue; - } - // if the type of dataBodyPart.getEntity() is BodyPartEntity, it is safe to read the part - // more than once. So we're not depriving the application of the data by consuming it here - String v = dataBodyPart.getValue(); + MultiPartHelper.collectBodyPart((FormDataBodyPart) bodyPart, map, filenames, filesContent); + } - String name = dataBodyPart.getName(); - List values = map.get(name); - if (values == null) { - values = new ArrayList<>(); - map.put(name, values); + if (map != null) { + Flow flow = callback.apply(reqCtx, map); + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, flow, "Blocked request (for MultiPartReaderServerSide/readMultiPart)"); + if (be != null) { + t = be; } + } - values.add(v); + if (filenames != null && !filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + if (t == null) { + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, filenamesFlow, "Blocked request (multipart file upload)"); + if (be != null) { + t = be; + } + } } - Flow flow = callback.apply(reqCtx, map); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (for MultiPartReaderClientSide/readFrom)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + if (filesContent != null && !filesContent.isEmpty()) { + Flow contentFlow = contentCallback.apply(reqCtx, filesContent); + if (t == null) { + BlockingException be = + MultiPartHelper.tryBlock( + reqCtx, contentFlow, "Blocked request (multipart file upload content)"); + if (be != null) { + t = be; + } } } } diff --git a/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/test/groovy/MultiPartHelperTest.groovy b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/test/groovy/MultiPartHelperTest.groovy new file mode 100644 index 00000000000..23feea4cc58 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/test/groovy/MultiPartHelperTest.groovy @@ -0,0 +1,282 @@ +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.api.gateway.BlockResponseFunction +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.internal.TraceSegment +import datadog.trace.instrumentation.jersey3.MultiPartHelper +import org.glassfish.jersey.media.multipart.FormDataBodyPart +import org.glassfish.jersey.media.multipart.FormDataContentDisposition +import spock.lang.Specification + +import jakarta.ws.rs.core.MediaType + +class MultiPartHelperTest extends Specification { + + // collectBodyPart — body map + + def "text/plain part is added to body map"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'field' + bodyPart.getValue() >> 'value' + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map == [field: ['value']] + } + + def "non-text/plain part is not added to body map"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map.isEmpty() + } + + def "null body map is skipped without error"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getFormDataContentDisposition() >> null + + expect: + MultiPartHelper.collectBodyPart(bodyPart, null, null, null) + } + + def "multiple values for same field are accumulated"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'tag' + bodyPart.getValue() >>> ['a', 'b'] + bodyPart.getFormDataContentDisposition() >> null + def map = [:] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + MultiPartHelper.collectBodyPart(bodyPart, map, null, null) + + then: + map == [tag: ['a', 'b']] + } + + // collectBodyPart — filenames + + def "filename is added to list when present"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + def filenames = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, null) + + then: + filenames == ['report.php'] + } + + def "null filenames list is skipped without error"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'f' + bodyPart.getValue() >> 'v' + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.collectBodyPart(bodyPart, [:], null, null) + } + + def "text/plain part with filename populates both body map and filename list"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'upload.txt' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.TEXT_PLAIN_TYPE + bodyPart.getName() >> 'file' + bodyPart.getValue() >> 'content' + bodyPart.getFormDataContentDisposition() >> cd + def map = [:] + def filenames = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, map, filenames, null) + + then: + map == [file: ['content']] + filenames == ['upload.txt'] + } + + // collectBodyPart — files content + + def "empty filename adds to content but not filenames"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> '' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('data'.bytes) + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content == ['data'] + } + + def "null filename (no filename attr) skips both filenames and content"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> null + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content.isEmpty() + } + + def "non-empty filename adds to both filenames and content"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> 'report.php' + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> cd + bodyPart.getEntityAs(InputStream) >> new ByteArrayInputStream('> MediaType.APPLICATION_OCTET_STREAM_TYPE + bodyPart.getFormDataContentDisposition() >> { throw new IllegalArgumentException("bad CD") } + def filenames = [] + def content = [] + + when: + MultiPartHelper.collectBodyPart(bodyPart, null, filenames, content) + + then: + filenames.isEmpty() + content.isEmpty() + noExceptionThrown() + } + + def "MAX_FILES_TO_INSPECT limits number of content entries"() { + given: + def parts = (1..MultiPartHelper.MAX_FILES_TO_INSPECT + 2).collect { i -> + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> "f${i}.bin" + def bp = Mock(FormDataBodyPart) + bp.getMediaType() >> MediaType.APPLICATION_OCTET_STREAM_TYPE + bp.getFormDataContentDisposition() >> cd + bp.getEntityAs(InputStream) >> new ByteArrayInputStream("content${i}".bytes) + bp + } + def content = [] + + when: + parts.each { MultiPartHelper.collectBodyPart(it, null, null, content) } + + then: + content.size() == MultiPartHelper.MAX_FILES_TO_INSPECT + } + + // tryBlock + + def "tryBlock returns null when flow action is not a blocking action"() { + given: + Flow flow = Stub(Flow) + flow.getAction() >> Flow.Action.Noop.INSTANCE + RequestContext ctx = Stub(RequestContext) + + expect: + MultiPartHelper.tryBlock(ctx, flow, 'msg') == null + } + + def "tryBlock returns BlockingException with provided message when brf commits response"() { + given: + def segment = Stub(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Stub(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + def result = MultiPartHelper.tryBlock(ctx, flow, 'blocked!') + + then: + result instanceof BlockingException + result.message == 'blocked!' + } + + def "tryBlock calls tryCommitBlockingResponse and effectivelyBlocked"() { + given: + def segment = Mock(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Mock(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + MultiPartHelper.tryBlock(ctx, flow, 'msg') + + then: + 1 * brf.tryCommitBlockingResponse(segment, rba) + 1 * segment.effectivelyBlocked() + } + + def "tryBlock returns null when brf is null despite blocking action"() { + given: + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> null + + expect: + MultiPartHelper.tryBlock(ctx, flow, 'msg') == null + } +} diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/HttpPostRequestDecoderInstrumentation.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/HttpPostRequestDecoderInstrumentation.java index d1509ce498b..461f2f93572 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/HttpPostRequestDecoderInstrumentation.java +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/HttpPostRequestDecoderInstrumentation.java @@ -6,25 +6,18 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.muzzle.Reference; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import io.netty.handler.codec.http.EmptyHttpHeaders; -import io.netty.handler.codec.http.multipart.Attribute; -import io.netty.handler.codec.http.multipart.FileUpload; -import io.netty.handler.codec.http.multipart.InterfaceHttpData; import io.netty.handler.codec.http.multipart.InterfaceHttpPostRequestDecoder; -import java.io.IOException; -import java.lang.reflect.UndeclaredThrowableException; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -58,6 +51,7 @@ public Reference[] additionalMuzzleReferences() { Reference.EXPECTS_NON_STATIC, "currentStatus", "Lio/netty/handler/codec/http/multipart/HttpPostRequestDecoder$MultiPartStatus;") + .withField(new String[0], Reference.EXPECTS_NON_STATIC, "isLastChunk", "Z") .build(), new Reference.Builder("io.netty.handler.codec.http.multipart.HttpPostStandardRequestDecoder") .withField( @@ -65,10 +59,18 @@ public Reference[] additionalMuzzleReferences() { Reference.EXPECTS_NON_STATIC, "currentStatus", "Lio/netty/handler/codec/http/multipart/HttpPostRequestDecoder$MultiPartStatus;") + .withField(new String[0], Reference.EXPECTS_NON_STATIC, "isLastChunk", "Z") .build() }; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".NettyMultipartHelper", + }; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -82,72 +84,66 @@ static class ParseBodyAdvice { static void after( @Advice.This InterfaceHttpPostRequestDecoder thiz, @Advice.FieldValue("currentStatus") Enum currentStatus, + @Advice.FieldValue("isLastChunk") boolean isLastChunk, @ActiveRequestContext RequestContext requestContext, @Advice.Thrown(readOnly = false) Throwable thr) { - if (!currentStatus.name().equals("EPILOGUE")) { - return; + String statusName = currentStatus.name(); + if (!statusName.equals("EPILOGUE")) { + // For multipart decoders, the PREEPILOGUE→EPILOGUE transition requires a second + // parseBody() call that never comes when the full request arrives in one shot. + // Fire on PREEPILOGUE + isLastChunk to handle that case. + if (!statusName.equals("PREEPILOGUE") || !isLastChunk) { + return; + } } CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); BiFunction> callback = cbp.getCallback(EVENTS.requestBodyProcessed()); - if (callback == null) { + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + + if (callback == null && filenamesCb == null && contentCb == null) { return; } - RuntimeException exc = null; - - Map> attributes = new LinkedHashMap<>(); - List filenames = new ArrayList<>(); - for (InterfaceHttpData data : thiz.getBodyHttpDatas()) { - if (data.getHttpDataType() == InterfaceHttpData.HttpDataType.Attribute) { - String name = data.getName(); - List values = attributes.get(name); - if (values == null) { - attributes.put(name, values = new ArrayList<>(1)); - } - - try { - values.add(((Attribute) data).getValue()); - } catch (IOException e) { - exc = new UndeclaredThrowableException(e); - } - } else if (data.getHttpDataType() == InterfaceHttpData.HttpDataType.FileUpload) { - String filename = ((FileUpload) data).getFilename(); - if (filename != null && !filename.isEmpty()) { - filenames.add(filename); - } - } + Map> attributes = callback != null ? new LinkedHashMap<>() : null; + List filenames = filenamesCb != null ? new ArrayList<>() : null; + List filesContent = contentCb != null ? new ArrayList<>() : null; + + RuntimeException exc = + NettyMultipartHelper.collectBodyData( + thiz.getBodyHttpDatas(), attributes, filenames, filesContent); + + if (callback != null) { + // effectivelyBlocked() is intentionally absent: tryCommitBlockingResponse finishes + // the span synchronously in this Netty path; calling it on a finished span throws. + thr = + NettyMultipartHelper.tryBlock( + requestContext, + callback.apply(requestContext, attributes), + "Blocked request (multipart/urlencoded post data)"); } - Flow flow = callback.apply(requestContext, attributes); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction brf = requestContext.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); + if (filenames != null && !filenames.isEmpty()) { + Flow filenamesFlow = filenamesCb.apply(requestContext, filenames); + if (thr == null) { + thr = + NettyMultipartHelper.tryBlock( + requestContext, filenamesFlow, "Blocked request (multipart file upload)"); } - thr = new BlockingException("Blocked request (multipart/urlencoded post data)"); } - if (!filenames.isEmpty()) { - BiFunction, Flow> filenamesCb = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (filenamesCb != null) { - Flow filenamesFlow = filenamesCb.apply(requestContext, filenames); - Flow.Action filenamesAction = filenamesFlow.getAction(); - if (thr == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = - (Flow.Action.RequestBlockingAction) filenamesAction; - BlockResponseFunction brf = requestContext.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba); - } - thr = new BlockingException("Blocked request (multipart file upload)"); - } - } + if (thr == null && filesContent != null && !filesContent.isEmpty()) { + thr = + NettyMultipartHelper.tryBlock( + requestContext, + contentCb.apply(requestContext, filesContent), + "Blocked request (multipart file upload content)"); } if (exc != null) { diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java new file mode 100644 index 00000000000..b268216f2aa --- /dev/null +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyMultipartHelper.java @@ -0,0 +1,100 @@ +package datadog.trace.instrumentation.netty41; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.http.MultipartContentDecoder; +import io.netty.buffer.ByteBuf; +import io.netty.handler.codec.http.multipart.Attribute; +import io.netty.handler.codec.http.multipart.FileUpload; +import io.netty.handler.codec.http.multipart.InterfaceHttpData; +import java.io.FileInputStream; +import java.io.IOException; +import java.lang.reflect.UndeclaredThrowableException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public final class NettyMultipartHelper { + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + /** + * Iterates multipart body parts populating the provided output collections. Pass {@code null} for + * any collection to skip that category. Returns any {@link IOException} encountered reading an + * attribute value, wrapped as {@link UndeclaredThrowableException}, or {@code null} if none. + */ + public static RuntimeException collectBodyData( + List parts, + Map> attributes, + List filenames, + List filesContent) { + RuntimeException exc = null; + for (InterfaceHttpData data : parts) { + if (attributes != null + && data.getHttpDataType() == InterfaceHttpData.HttpDataType.Attribute) { + String name = data.getName(); + List values = attributes.get(name); + if (values == null) { + attributes.put(name, values = new ArrayList<>(1)); + } + try { + values.add(((Attribute) data).getValue()); + } catch (IOException e) { + exc = new UndeclaredThrowableException(e); + } + } else if (data.getHttpDataType() == InterfaceHttpData.HttpDataType.FileUpload) { + FileUpload fileUpload = (FileUpload) data; + String filename = fileUpload.getFilename(); + if (filenames != null && filename != null && !filename.isEmpty()) { + filenames.add(filename); + } + if (filesContent != null && filesContent.size() < MAX_FILES_TO_INSPECT) { + filesContent.add(readContent(fileUpload)); + } + } + } + return exc; + } + + public static String readContent(FileUpload fileUpload) { + try { + if (fileUpload.isInMemory()) { + ByteBuf buf = fileUpload.getByteBuf(); + int length = Math.min(MAX_CONTENT_BYTES, buf.readableBytes()); + byte[] bytes = new byte[length]; + buf.getBytes(buf.readerIndex(), bytes); + return MultipartContentDecoder.decodeBytes(bytes, length, fileUpload.getContentType()); + } else { + try (FileInputStream fis = new FileInputStream(fileUpload.getFile())) { + return MultipartContentDecoder.readInputStream( + fis, MAX_CONTENT_BYTES, fileUpload.getContentType()); + } + } + } catch (Exception ignored) { + return ""; + } + } + + /** + * Checks if the flow action is a blocking action and, if so, commits the blocking response. + * Returns a {@link BlockingException} to be re-thrown by the advice, or {@code null} if no + * blocking action was taken. + */ + public static BlockingException tryBlock(RequestContext ctx, Flow flow, String message) { + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + return new BlockingException(message); + } + } + return null; + } + + private NettyMultipartHelper() {} +} diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy index c9b37933033..1c4831d0ce5 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy @@ -27,6 +27,7 @@ import io.netty.handler.codec.http.HttpResponseStatus import io.netty.handler.codec.http.HttpServerCodec import io.netty.handler.codec.http.multipart.Attribute import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder +import io.netty.handler.codec.http.multipart.InterfaceHttpData import io.netty.handler.codec.http.websocketx.BinaryWebSocketFrame import io.netty.handler.codec.http.websocketx.CloseWebSocketFrame import io.netty.handler.codec.http.websocketx.ContinuationWebSocketFrame @@ -36,6 +37,7 @@ import io.netty.handler.logging.LogLevel import io.netty.handler.logging.LoggingHandler import io.netty.util.CharsetUtil +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION @@ -73,7 +75,7 @@ abstract class Netty41ServerTest extends HttpServerTest { ChannelPipeline pipeline = ch.pipeline() pipeline.addFirst("logger", LOGGING_HANDLER) pipeline.addLast(new HttpServerCodec()) - pipeline.addLast(new HttpObjectAggregator(1024)) + pipeline.addLast(new HttpObjectAggregator(1024 * 1024)) pipeline.addLast(new WebSocketServerProtocolHandler("/websocket")) pipeline.addLast([ channelRead0 : { ChannelHandlerContext ctx, msg -> @@ -133,6 +135,31 @@ abstract class Netty41ServerTest extends HttpServerTest { response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status), content) } break + case BODY_MULTIPART: + if (msg instanceof FullHttpRequest) { + HttpPostRequestDecoder decoder = new HttpPostRequestDecoder( + new HttpRequest() { + @Delegate + HttpRequest delegate = request + }) + + Map m + try { + decoder.offer(msg) + + m = decoder.bodyHttpDatas + .findAll { it.httpDataType == InterfaceHttpData.HttpDataType.Attribute } + .collectEntries { d -> [d.name, [((Attribute) d).value]] } + } finally { + try { + decoder.destroy() + } catch (Exception ignored) {} + } + + content = Unpooled.copiedBuffer(m as String, CharsetUtil.UTF_8) + response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status), content) + } + break case REDIRECT: response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status)) response.headers().set(HttpHeaderNames.LOCATION, endpoint.body) @@ -270,6 +297,21 @@ abstract class Netty41ServerTest extends HttpServerTest { true } + @Override + boolean testBodyMultipart() { + true + } + + @Override + boolean testBodyFilenames() { + true + } + + @Override + boolean testBodyFilesContent() { + true + } + @Override boolean testBlocking() { true diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/NettyMultipartHelperTest.groovy b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/NettyMultipartHelperTest.groovy new file mode 100644 index 00000000000..c7b5f388900 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/NettyMultipartHelperTest.groovy @@ -0,0 +1,482 @@ +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.api.gateway.BlockResponseFunction +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.internal.TraceSegment +import datadog.trace.instrumentation.netty41.NettyMultipartHelper +import io.netty.buffer.Unpooled +import io.netty.handler.codec.http.multipart.Attribute +import io.netty.handler.codec.http.multipart.FileUpload +import io.netty.handler.codec.http.multipart.InterfaceHttpData +import spock.lang.Specification +import spock.lang.TempDir + +import java.lang.reflect.UndeclaredThrowableException +import java.nio.charset.StandardCharsets +import java.nio.file.Path + +class NettyMultipartHelperTest extends Specification { + + @TempDir + Path tempDir + + // ========================================================= + // readContent — in-memory path + // ========================================================= + + void 'readContent returns content from in-memory FileUpload'() { + given: + def upload = inMemoryUpload('hello world') + + expect: + NettyMultipartHelper.readContent(upload) == 'hello world' + } + + void 'readContent truncates in-memory content at MAX_CONTENT_BYTES'() { + given: + def upload = inMemoryUpload('X' * inputSize) + + when: + def result = NettyMultipartHelper.readContent(upload) + + then: + result.length() == expectedSize + + where: + inputSize | expectedSize + 13 | 13 + NettyMultipartHelper.MAX_CONTENT_BYTES - 1 | NettyMultipartHelper.MAX_CONTENT_BYTES - 1 + NettyMultipartHelper.MAX_CONTENT_BYTES | NettyMultipartHelper.MAX_CONTENT_BYTES + NettyMultipartHelper.MAX_CONTENT_BYTES + 500 | NettyMultipartHelper.MAX_CONTENT_BYTES + } + + void 'readContent returns empty string for empty in-memory content'() { + given: + def upload = inMemoryUpload('') + + expect: + NettyMultipartHelper.readContent(upload) == '' + } + + void 'readContent does not advance readerIndex of the underlying ByteBuf'() { + given: + def content = 'sensitive data' + def upload = inMemoryUpload(content) + def buf = upload.getByteBuf() + def indexBefore = buf.readerIndex() + + when: + NettyMultipartHelper.readContent(upload) + + then: + buf.readerIndex() == indexBefore + } + + // ========================================================= + // readContent — disk-backed path + // ========================================================= + + void 'readContent returns content from disk-backed FileUpload'() { + given: + def upload = diskBackedUpload('hello from disk') + + expect: + NettyMultipartHelper.readContent(upload) == 'hello from disk' + } + + void 'readContent truncates disk-backed content at MAX_CONTENT_BYTES'() { + given: + def upload = diskBackedUpload('Y' * inputSize) + + when: + def result = NettyMultipartHelper.readContent(upload) + + then: + result.length() == expectedSize + + where: + inputSize | expectedSize + 13 | 13 + NettyMultipartHelper.MAX_CONTENT_BYTES - 1 | NettyMultipartHelper.MAX_CONTENT_BYTES - 1 + NettyMultipartHelper.MAX_CONTENT_BYTES | NettyMultipartHelper.MAX_CONTENT_BYTES + NettyMultipartHelper.MAX_CONTENT_BYTES + 500 | NettyMultipartHelper.MAX_CONTENT_BYTES + } + + void 'readContent returns empty string for empty disk-backed file'() { + given: + def upload = diskBackedUpload('') + + expect: + NettyMultipartHelper.readContent(upload) == '' + } + + // ========================================================= + // readContent — charset decoding + // ========================================================= + + void 'readContent uses Content-Type charset for in-memory content'() { + given: + def text = 'héllo wörld' + def buf = Unpooled.copiedBuffer(text, StandardCharsets.UTF_8) + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> true + upload.getByteBuf() >> buf + upload.getContentType() >> 'text/plain; charset=UTF-8' + + expect: + NettyMultipartHelper.readContent(upload) == text + } + + void 'readContent uses Content-Type charset for disk-backed content'() { + given: + def text = 'héllo wörld' + def file = tempDir.resolve('upload.bin').toFile() + file.bytes = text.getBytes(StandardCharsets.UTF_8) + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> false + upload.getFile() >> file + upload.getContentType() >> 'text/plain; charset=UTF-8' + + expect: + NettyMultipartHelper.readContent(upload) == text + } + + // ========================================================= + // readContent — error handling + // ========================================================= + + void 'readContent returns empty string when getByteBuf throws'() { + given: + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> true + upload.getByteBuf() >> { throw new RuntimeException('simulated error') } + + expect: + NettyMultipartHelper.readContent(upload) == '' + } + + void 'readContent returns empty string when getFile throws'() { + given: + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> false + upload.getFile() >> { throw new IOException('simulated error') } + + expect: + NettyMultipartHelper.readContent(upload) == '' + } + + // ========================================================= + // collectBodyData — empty / null inputs + // ========================================================= + + void 'collectBodyData returns null and leaves collections empty when parts list is empty'() { + given: + def attributes = [:] + def filenames = [] + def filesContent = [] + + expect: + NettyMultipartHelper.collectBodyData([], attributes, filenames, filesContent) == null + attributes.isEmpty() + filenames.isEmpty() + filesContent.isEmpty() + } + + // ========================================================= + // collectBodyData — attribute handling + // ========================================================= + + void 'collectBodyData collects attribute value into map'() { + given: + def attr = attribute('key', 'value') + def attributes = [:] + + when: + NettyMultipartHelper.collectBodyData([attr], attributes, null, null) + + then: + attributes == [key: ['value']] + } + + void 'collectBodyData accumulates multiple values for the same attribute key'() { + given: + def parts = [attribute('k', 'v1'), attribute('k', 'v2')] + def attributes = [:] + + when: + NettyMultipartHelper.collectBodyData(parts, attributes, null, null) + + then: + attributes == [k: ['v1', 'v2']] + } + + void 'collectBodyData skips attribute parts when attributes map is null'() { + given: + def attr = attribute('key', 'value') + def filenames = [] + + when: + NettyMultipartHelper.collectBodyData([attr], null, filenames, null) + + then: + filenames.isEmpty() + } + + void 'collectBodyData wraps IOException from getValue as UndeclaredThrowableException'() { + given: + def cause = new IOException('disk error') + Attribute attr = Stub(Attribute) + attr.getHttpDataType() >> InterfaceHttpData.HttpDataType.Attribute + attr.getName() >> 'k' + attr.getValue() >> { throw cause } + def attributes = [:] + + when: + def exc = NettyMultipartHelper.collectBodyData([attr], attributes, null, null) + + then: + exc instanceof UndeclaredThrowableException + exc.cause.is(cause) + } + + void 'collectBodyData continues collecting after an IOException from getValue'() { + given: + Attribute failing = Stub(Attribute) + failing.getHttpDataType() >> InterfaceHttpData.HttpDataType.Attribute + failing.getName() >> 'bad' + failing.getValue() >> { throw new IOException('disk error') } + + def good = attribute('good', 'ok') + def attributes = [:] + + when: + NettyMultipartHelper.collectBodyData([failing, good], attributes, null, null) + + then: + attributes == [bad: [], good: ['ok']] + } + + // ========================================================= + // collectBodyData — filename handling + // ========================================================= + + void 'collectBodyData collects non-empty filename from file upload'() { + given: + def filenames = [] + + when: + NettyMultipartHelper.collectBodyData([fileUploadPart('report.pdf', '')], null, filenames, null) + + then: + filenames == ['report.pdf'] + } + + void 'collectBodyData skips empty filename from file upload'() { + given: + def filenames = [] + + when: + NettyMultipartHelper.collectBodyData([fileUploadPart('', '')], null, filenames, null) + + then: + filenames.isEmpty() + } + + void 'collectBodyData skips null filename from file upload'() { + given: + FileUpload upload = Stub(FileUpload) + upload.getHttpDataType() >> InterfaceHttpData.HttpDataType.FileUpload + upload.getFilename() >> null + upload.isInMemory() >> true + upload.getByteBuf() >> Unpooled.EMPTY_BUFFER + def filenames = [] + + when: + NettyMultipartHelper.collectBodyData([upload], null, filenames, null) + + then: + filenames.isEmpty() + } + + void 'collectBodyData skips filenames when filenames list is null'() { + given: + def filesContent = [] + + when: + NettyMultipartHelper.collectBodyData( + [fileUploadPart('report.pdf', 'data')], null, null, filesContent) + + then: + filesContent == ['data'] + } + + // ========================================================= + // collectBodyData — content handling + // ========================================================= + + void 'collectBodyData reads file content into filesContent list'() { + given: + def filesContent = [] + + when: + NettyMultipartHelper.collectBodyData( + [fileUploadPart('f.txt', 'hello')], null, null, filesContent) + + then: + filesContent == ['hello'] + } + + void 'collectBodyData skips content when filesContent list is null'() { + given: + def filenames = [] + + when: + NettyMultipartHelper.collectBodyData( + [fileUploadPart('f.txt', 'hello')], null, filenames, null) + + then: + filenames == ['f.txt'] + } + + void 'collectBodyData stops reading content after MAX_FILES_TO_INSPECT but keeps collecting filenames'() { + given: + def max = NettyMultipartHelper.MAX_FILES_TO_INSPECT + def parts = (1..max + 1).collect { i -> fileUploadPart("file${i}.txt", "content${i}") } + def filenames = [] + def filesContent = [] + + when: + NettyMultipartHelper.collectBodyData(parts, null, filenames, filesContent) + + then: + filesContent.size() == max + filenames.size() == max + 1 + } + + // ========================================================= + // collectBodyData — mixed parts + // ========================================================= + + void 'collectBodyData handles mixed attributes and file uploads in one pass'() { + given: + def parts = [ + attribute('field1', 'val1'), + fileUploadPart('upload.txt', 'file-body'), + attribute('field2', 'val2'), + ] + def attributes = [:] + def filenames = [] + def filesContent = [] + + when: + def exc = NettyMultipartHelper.collectBodyData(parts, attributes, filenames, filesContent) + + then: + exc == null + attributes == [field1: ['val1'], field2: ['val2']] + filenames == ['upload.txt'] + filesContent == ['file-body'] + } + + // ========================================================= + // tryBlock + // ========================================================= + + void 'tryBlock returns null when flow action is not a blocking action'() { + given: + Flow flow = Stub(Flow) + flow.getAction() >> Flow.Action.Noop.INSTANCE + RequestContext ctx = Stub(RequestContext) + + expect: + NettyMultipartHelper.tryBlock(ctx, flow, 'msg') == null + } + + void 'tryBlock returns BlockingException with provided message when brf commits response'() { + given: + def segment = Stub(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Stub(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + def result = NettyMultipartHelper.tryBlock(ctx, flow, 'blocked!') + + then: + result instanceof BlockingException + result.message == 'blocked!' + } + + void 'tryBlock calls tryCommitBlockingResponse on brf with segment and rba'() { + given: + def segment = Stub(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Mock(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + NettyMultipartHelper.tryBlock(ctx, flow, 'msg') + + then: + 1 * brf.tryCommitBlockingResponse(segment, rba) + } + + void 'tryBlock returns null when brf is null despite blocking action'() { + given: + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> null + + expect: + NettyMultipartHelper.tryBlock(ctx, flow, 'msg') == null + } + + // ========================================================= + // helpers + // ========================================================= + + private FileUpload inMemoryUpload(String content) { + def buf = Unpooled.copiedBuffer(content, StandardCharsets.ISO_8859_1) + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> true + upload.getByteBuf() >> buf + return upload + } + + private FileUpload diskBackedUpload(String content) { + def file = tempDir.resolve('upload.bin').toFile() + file.bytes = content.getBytes(StandardCharsets.ISO_8859_1) + FileUpload upload = Stub(FileUpload) + upload.isInMemory() >> false + upload.getFile() >> file + return upload + } + + private Attribute attribute(String name, String value) { + Attribute attr = Stub(Attribute) + attr.getHttpDataType() >> InterfaceHttpData.HttpDataType.Attribute + attr.getName() >> name + attr.getValue() >> value + return attr + } + + private FileUpload fileUploadPart(String filename, String content) { + def buf = Unpooled.copiedBuffer(content, StandardCharsets.ISO_8859_1) + FileUpload upload = Stub(FileUpload) + upload.getHttpDataType() >> InterfaceHttpData.HttpDataType.FileUpload + upload.getFilename() >> filename + upload.isInMemory() >> true + upload.getByteBuf() >> buf + return upload + } +} 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 1163d35202c..eb6d7b3dd75 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 @@ -11,7 +11,6 @@ import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.gateway.BlockResponseFunction; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -45,6 +44,11 @@ public String instrumentedType() { return "org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataReader"; } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".MultipartHelper"}; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -72,28 +76,60 @@ static void after( CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); BiFunction> callback = cbp.getCallback(EVENTS.requestBodyProcessed()); - if (callback == null) { + BiFunction, Flow> filenamesCallback = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCallback = + cbp.getCallback(EVENTS.requestFilesContent()); + if (callback == null && filenamesCallback == null && contentCallback == null) { return; } - 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()); + 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()); + } + } + + Flow flow = callback.apply(reqCtx, m); + BlockingException be = + MultipartHelper.tryBlock( + reqCtx, flow, "Blocked request (for MultipartFormDataInput/readFrom)"); + if (be != null) { + t = be; + } + } + + if (filenamesCallback != null) { + List filenames = MultipartHelper.collectFilenames(ret); + if (!filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + if (t == null) { + BlockingException be = + MultipartHelper.tryBlock( + reqCtx, filenamesFlow, "Blocked request (multipart file upload)"); + if (be != null) { + t = be; + } + } } } - Flow flow = callback.apply(reqCtx, m); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); - if (blockResponseFunction != null) { - blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - t = new BlockingException("Blocked request (for MultipartFormDataInput/readFrom)"); - reqCtx.getTraceSegment().effectivelyBlocked(); + if (contentCallback != null) { + List filesContent = MultipartHelper.collectFilesContent(ret); + if (!filesContent.isEmpty()) { + Flow contentFlow = contentCallback.apply(reqCtx, filesContent); + if (t == null) { + BlockingException be = + MultipartHelper.tryBlock( + reqCtx, contentFlow, "Blocked request (multipart file upload content)"); + if (be != null) { + t = 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 new file mode 100644 index 00000000000..5534f066a08 --- /dev/null +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java @@ -0,0 +1,183 @@ +package datadog.trace.instrumentation.resteasy; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +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.List; +import java.util.Map; +import org.jboss.resteasy.plugins.providers.multipart.InputPart; +import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataInput; + +public final class MultipartHelper { + + public static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + public static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + private MultipartHelper() {} + + // Reflection avoids a bytecode ref to MultivaluedMap (javax→jakarta in RESTEasy 6) + private static final Method GET_HEADERS; + + static { + Method m = null; + try { + m = InputPart.class.getMethod("getHeaders"); + } catch (NoSuchMethodException ignored) { + } + GET_HEADERS = m; + } + + public static List collectFilenames(MultipartFormDataInput ret) { + List filenames = new ArrayList<>(); + if (GET_HEADERS == null) { + return filenames; + } + for (Map.Entry> e : ret.getFormDataMap().entrySet()) { + for (InputPart inputPart : e.getValue()) { + List cdHeaders; + try { + @SuppressWarnings("unchecked") + Map> headers = + (Map>) GET_HEADERS.invoke(inputPart); + cdHeaders = headers != null ? headers.get("Content-Disposition") : null; + } catch (Exception ignored) { + continue; + } + if (cdHeaders == null || cdHeaders.isEmpty()) { + continue; + } + String filename = filenameFromContentDisposition(cdHeaders.get(0)); + if (filename != null) { + filenames.add(filename); + } + } + } + return filenames; + } + + public static List collectFilesContent(MultipartFormDataInput ret) { + List contents = new ArrayList<>(); + if (GET_HEADERS == null) { + return contents; + } + for (Map.Entry> e : ret.getFormDataMap().entrySet()) { + for (InputPart inputPart : e.getValue()) { + if (contents.size() >= MAX_FILES_TO_INSPECT) { + return contents; + } + Map> headers; + try { + @SuppressWarnings("unchecked") + Map> h = (Map>) GET_HEADERS.invoke(inputPart); + headers = h; + } catch (Exception ignored) { + continue; + } + if (headers == null) { + continue; + } + List cdHeaders = headers.get("Content-Disposition"); + if (cdHeaders == null || cdHeaders.isEmpty()) { + continue; + } + // rawFilenameFromContentDisposition returns null if filename attr absent, + // otherwise returns the value (possibly empty) — both cases warrant content inspection + if (rawFilenameFromContentDisposition(cdHeaders.get(0)) == null) { + continue; + } + List ctHeaders = headers.get("Content-Type"); + String contentType = (ctHeaders != null && !ctHeaders.isEmpty()) ? ctHeaders.get(0) : null; + contents.add(readContent(inputPart, contentType)); + } + } + return contents; + } + + public static BlockingException tryBlock(RequestContext ctx, Flow flow, String message) { + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse(ctx.getTraceSegment(), rba); + BlockingException be = new BlockingException(message); + ctx.getTraceSegment().effectivelyBlocked(); + return be; + } + } + return null; + } + + 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) { + return ""; + } + } + + // Quote-aware: semicolons inside quoted filenames (e.g. filename="a;b.php") are not separators. + // Outer loop: i advances to each ';' (skipping quoted strings to avoid treating their contents + // as delimiters), then past MIME linear whitespace (SP/HT) to the start of the parameter name. + // j is a lookahead used only to find '=' after optional whitespace without committing i until + // the parameter is confirmed to be "filename"; this avoids confusing "filename*" (RFC 5987) or + // other "filename"-prefixed parameter names with the plain "filename" parameter. + public static String filenameFromContentDisposition(String cd) { + String raw = rawFilenameFromContentDisposition(cd); + return (raw == null || raw.isEmpty()) ? null : raw; + } + + // Like filenameFromContentDisposition but returns "" for present-but-empty filename, + // and null only when the filename parameter is absent entirely. + static String rawFilenameFromContentDisposition(String cd) { + if (cd == null) return null; + int i = 0; + int len = cd.length(); + while (i < len) { + while (i < len && cd.charAt(i) != ';') { + if (cd.charAt(i) == '"') { + i++; + while (i < len && cd.charAt(i) != '"') { + if (cd.charAt(i) == '\\') i++; + i++; + } + } + i++; + } + if (i >= len) break; + i++; + while (i < len && (cd.charAt(i) == ' ' || cd.charAt(i) == '\t')) i++; + if (cd.regionMatches(true, i, "filename", 0, 8)) { + int j = i + 8; + while (j < len && (cd.charAt(j) == ' ' || cd.charAt(j) == '\t')) j++; + if (j < len && cd.charAt(j) == '=') { + i = j + 1; + while (i < len && (cd.charAt(i) == ' ' || cd.charAt(i) == '\t')) i++; + if (i >= len) return ""; + if (cd.charAt(i) == '"') { + i++; + StringBuilder sb = new StringBuilder(); + while (i < len && cd.charAt(i) != '"') { + if (cd.charAt(i) == '\\' && i + 1 < len) i++; // unescape + sb.append(cd.charAt(i++)); + } + return sb.toString(); + } else { + int start = i; + while (i < len && cd.charAt(i) != ';') i++; + return cd.substring(start, i).trim(); + } + } + } + } + return null; + } +} 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 new file mode 100644 index 00000000000..ab08b74d151 --- /dev/null +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy @@ -0,0 +1,275 @@ +import datadog.appsec.api.blocking.BlockingException +import datadog.trace.api.gateway.BlockResponseFunction +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.internal.TraceSegment +import datadog.trace.instrumentation.resteasy.MultipartHelper +import org.jboss.resteasy.plugins.providers.multipart.InputPart +import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataInput +import org.jboss.resteasy.specimpl.MultivaluedMapImpl +import spock.lang.Specification + +import java.lang.reflect.Method + +class MultipartHelperTest extends Specification { + + // rawFilenameFromContentDisposition (package-private, tested via reflection) + + private static String rawFilename(String cd) { + Method m = MultipartHelper.getDeclaredMethod('rawFilenameFromContentDisposition', String) + m.setAccessible(true) + return (String) m.invoke(null, [cd] as Object[]) + } + + def "rawFilenameFromContentDisposition returns null when filename attr absent"() { + expect: + rawFilename(cd) == null + + where: + cd << [null, 'form-data', 'form-data; name="field"', ''] + } + + def "rawFilenameFromContentDisposition returns empty string for filename with empty value"() { + expect: + rawFilename('form-data; filename=""') == '' + rawFilename('form-data; filename=') == '' + } + + def "rawFilenameFromContentDisposition returns the value for non-empty filename"() { + expect: + rawFilename('form-data; filename="report.php"') == 'report.php' + rawFilename('form-data; filename=report.php') == 'report.php' + } + + // filenameFromContentDisposition (public API) + + def "returns null when no filename parameter"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == null + + where: + cd << [ + null, + 'form-data', + 'form-data; name="field"', + 'form-data; name="field"; other=value', + '', + ] + } + + def "extracts unquoted filename"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + cd | expected + 'form-data; filename=report.php' | 'report.php' + 'form-data; name="f"; filename=upload.txt' | 'upload.txt' + 'attachment; filename=file.tar.gz' | 'file.tar.gz' + } + + def "extracts quoted filename"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + cd | expected + 'form-data; filename="report.php"' | 'report.php' + 'form-data; name="f"; filename="upload.txt"' | 'upload.txt' + } + + def "handles semicolons inside quoted filename"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + cd | expected + 'form-data; filename="report;.php"' | 'report;.php' + 'form-data; name="f"; filename="a;b;c.php"' | 'a;b;c.php' + 'form-data; filename="shell;evil.php"' | 'shell;evil.php' + } + + def "handles escaped quotes inside filename"() { + expect: + MultipartHelper.filenameFromContentDisposition('form-data; filename="file\\"name.php"') == 'file"name.php' + } + + def "returns null for empty filename value"() { + expect: + MultipartHelper.filenameFromContentDisposition('form-data; filename=""') == null + MultipartHelper.filenameFromContentDisposition('form-data; filename=') == null + } + + def "is case-insensitive for the filename parameter name"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == 'report.php' + + where: + cd << [ + 'form-data; FILENAME="report.php"', + 'form-data; Filename="report.php"', + 'form-data; fileName="report.php"', + ] + } + + def "handles MIME linear whitespace (tab) after semicolon"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + cd | expected + 'form-data; name="f";\tfilename="evil.php"' | 'evil.php' + 'form-data;\tfilename="evil.php"' | 'evil.php' + 'form-data; name="f";\t\tfilename="evil.php"' | 'evil.php' + } + + def "handles optional whitespace around the equals sign"() { + expect: + MultipartHelper.filenameFromContentDisposition(cd) == expected + + where: + cd | expected + 'form-data; filename ="report.php"' | 'report.php' + 'form-data; filename= "report.php"' | 'report.php' + 'form-data; filename = "report.php"' | 'report.php' + 'form-data; filename\t=\t"report.php"' | 'report.php' + 'form-data; name="f";\tfilename\t=\t"evil.php"' | 'evil.php' + } + + def "does not match filename* extended parameter as filename"() { + expect: + MultipartHelper.filenameFromContentDisposition("form-data; filename*=UTF-8''evil.php") == null + } + + // collectFilesContent + + private static MultivaluedMapImpl headers(String cd) { + def h = new MultivaluedMapImpl() + h.add('Content-Disposition', cd) + h + } + + def "collectFilesContent includes part with non-empty filename"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="file"; filename="report.php"') + part.getBody(_, _) >> new ByteArrayInputStream('malicious'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['file': [part]] + + when: + def result = MultipartHelper.collectFilesContent(ret) + + then: + result == ['malicious'] + } + + def "collectFilesContent includes part with empty filename (security fix)"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="file"; filename=""') + part.getBody(_, _) >> new ByteArrayInputStream('anonymous'.bytes) + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['file': [part]] + + when: + def result = MultipartHelper.collectFilesContent(ret) + + then: + result == ['anonymous'] + } + + def "collectFilesContent skips part without filename attribute"() { + given: + def part = Mock(InputPart) + part.getHeaders() >> headers('form-data; name="field"') + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['field': [part]] + + when: + def result = MultipartHelper.collectFilesContent(ret) + + then: + result.isEmpty() + } + + def "collectFilesContent respects MAX_FILES_TO_INSPECT limit"() { + given: + def parts = (1..MultipartHelper.MAX_FILES_TO_INSPECT + 2).collect { i -> + def p = Mock(InputPart) + p.getHeaders() >> headers("form-data; name=\"f${i}\"; filename=\"f${i}.bin\"") + p.getBody(_, _) >> new ByteArrayInputStream("content${i}".bytes) + p + } + def ret = Mock(MultipartFormDataInput) + ret.getFormDataMap() >> ['files': parts] + + when: + def result = MultipartHelper.collectFilesContent(ret) + + then: + result.size() == MultipartHelper.MAX_FILES_TO_INSPECT + } + + // tryBlock + + def "tryBlock returns null when flow action is not a blocking action"() { + given: + Flow flow = Stub(Flow) + flow.getAction() >> Flow.Action.Noop.INSTANCE + RequestContext ctx = Stub(RequestContext) + + expect: + MultipartHelper.tryBlock(ctx, flow, 'msg') == null + } + + def "tryBlock returns BlockingException with provided message when brf commits response"() { + given: + def segment = Stub(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Stub(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + def result = MultipartHelper.tryBlock(ctx, flow, 'blocked!') + + then: + result instanceof BlockingException + result.message == 'blocked!' + } + + def "tryBlock calls tryCommitBlockingResponse and effectivelyBlocked"() { + given: + def segment = Mock(TraceSegment) + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + BlockResponseFunction brf = Mock(BlockResponseFunction) + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> brf + ctx.getTraceSegment() >> segment + + when: + MultipartHelper.tryBlock(ctx, flow, 'msg') + + then: + 1 * brf.tryCommitBlockingResponse(segment, rba) + 1 * segment.effectivelyBlocked() + } + + def "tryBlock returns null when brf is null despite blocking action"() { + given: + def rba = Stub(Flow.Action.RequestBlockingAction) + Flow flow = Stub(Flow) + flow.getAction() >> rba + RequestContext ctx = Stub(RequestContext) + ctx.getBlockResponseFunction() >> null + + expect: + MultipartHelper.tryBlock(ctx, flow, 'msg') == null + } +} diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java index 0c41f59eaba..1ff255c2654 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParameterCollector.java @@ -1,6 +1,10 @@ package datadog.trace.instrumentation.tomcat7; +import datadog.trace.api.Config; +import datadog.trace.api.http.MultipartContentDecoder; +import java.io.InputStream; import java.lang.reflect.Method; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -20,6 +24,8 @@ public interface ParameterCollector { List getFilenames(); + List getContents(); + class ParameterCollectorNoop implements ParameterCollector { public static final ParameterCollector INSTANCE = new ParameterCollectorNoop(); @@ -46,11 +52,25 @@ public void addPart(Object part) {} public List getFilenames() { return Collections.emptyList(); } + + @Override + public List getContents() { + return Collections.emptyList(); + } } class ParameterCollectorImpl implements ParameterCollector { + private static final int MAX_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes(); + private static final int MAX_FILES_TO_INSPECT = Config.get().getAppSecMaxFileContentCount(); + + private final boolean inspectContent; private Map> map; private List filenames; + private List contents; + + public ParameterCollectorImpl(boolean inspectContent) { + this.inspectContent = inspectContent; + } public boolean isEmpty() { return map == null; @@ -91,12 +111,25 @@ public Map> getMap() { public void addPart(Object part) { try { String filename = getFilename(part); - if (filename != null && !filename.isEmpty()) { + // null means no filename parameter at all → form field, skip entirely. + // empty string means filename="" was sent → file upload without a name, still inspect. + if (filename == null) { + return; + } + if (!filename.isEmpty()) { if (filenames == null) { filenames = new ArrayList<>(); } filenames.add(filename); } + if (inspectContent) { + if (contents == null) { + contents = new ArrayList<>(); + } + if (contents.size() < MAX_FILES_TO_INSPECT) { + contents.add(readContent(part)); + } + } } catch (Throwable ignored) { } } @@ -106,20 +139,71 @@ public List getFilenames() { return filenames != null ? filenames : Collections.emptyList(); } - private static String getFilename(Object part) { - // Try getSubmittedFileName() first — Servlet 3.1+ / Tomcat 8+ (both javax and jakarta) + @Override + public List getContents() { + return contents != null ? contents : Collections.emptyList(); + } + + // Entry caches (class → method[]) stored as a single volatile write for safe publication. + // methods[0]=getInputStream, methods[1]=getContentType + // Keyed by Part concrete class; re-resolved when the class changes (different Tomcat version). + private static volatile Map.Entry, Method[]> cachedContentMethodsEntry; + private static volatile Map.Entry, Method> cachedFilenameEntry; + + private static String readContent(Object part) { try { - Method m = part.getClass().getMethod("getSubmittedFileName"); - return (String) m.invoke(part); + Class partClass = part.getClass(); + Map.Entry, Method[]> entry = cachedContentMethodsEntry; + Method[] methods; + if (entry == null || entry.getKey() != partClass) { + methods = + new Method[] { + partClass.getMethod("getInputStream"), partClass.getMethod("getContentType") + }; + cachedContentMethodsEntry = new AbstractMap.SimpleImmutableEntry<>(partClass, methods); + } else { + methods = entry.getValue(); + } + String contentType = (String) methods[1].invoke(part); + try (InputStream is = (InputStream) methods[0].invoke(part)) { + return MultipartContentDecoder.readInputStream(is, MAX_CONTENT_BYTES, contentType); + } } catch (Exception ignored) { + return ""; + } + } + + private static String getFilename(Object part) { + Class partClass = part.getClass(); + Map.Entry, Method> entry = cachedFilenameEntry; + Method m; + if (entry == null || entry.getKey() != partClass) { + m = null; + // Try getSubmittedFileName() first — Servlet 3.1+ / Tomcat 8+ (both javax and jakarta) + try { + m = partClass.getMethod("getSubmittedFileName"); + } catch (Exception ignored) { + } + if (m == null) { + // Fall back to getFilename() — Tomcat 7 ApplicationPart specific + try { + m = partClass.getMethod("getFilename"); + } catch (Exception ignored) { + } + } + // null value in the entry means "no filename method on this class" + cachedFilenameEntry = new AbstractMap.SimpleImmutableEntry<>(partClass, m); + } else { + m = entry.getValue(); + } + if (m == null) { + return null; } - // Fall back to getFilename() — Tomcat 7 ApplicationPart specific try { - Method m = part.getClass().getMethod("getFilename"); return (String) m.invoke(part); } catch (Exception ignored) { + return null; } - return null; } } } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java index c249db9839a..72feb1a086a 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java @@ -86,7 +86,12 @@ static void before( RequestContext requestContext = agentSpan.getRequestContext(); if (requestContext != null && requestContext.getData(RequestContextSlot.APPSEC) != null) { reqCtx = requestContext; - collector = new ParameterCollector.ParameterCollectorImpl(); + boolean inspectContent = + AgentTracer.get() + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.requestFilesContent()) + != null; + collector = new ParameterCollector.ParameterCollectorImpl(inspectContent); return; } } @@ -117,9 +122,8 @@ static void after( BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); if (blockResponseFunction != null) { blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); - if (t == null) { - t = new BlockingException("Blocked request (for Request/parseParts)"); - } + t = new BlockingException("Blocked request (for Request/parseParts)"); + reqCtx.getTraceSegment().effectivelyBlocked(); } } } @@ -138,8 +142,31 @@ static void after( BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); if (brf != null) { brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } + } + } + + if (t == null) { + List contents = collector.getContents(); + if (!contents.isEmpty()) { + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (contentCb != null) { + Flow contentFlow = contentCb.apply(reqCtx, contents); + 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 content)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } - t = new BlockingException("Blocked request (multipart file upload)"); } } } diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/groovy/datadog/trace/instrumentation/tomcat7/ParameterCollectorImplTest.groovy b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/groovy/datadog/trace/instrumentation/tomcat7/ParameterCollectorImplTest.groovy new file mode 100644 index 00000000000..456a62122b1 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-7.0/src/test/groovy/datadog/trace/instrumentation/tomcat7/ParameterCollectorImplTest.groovy @@ -0,0 +1,276 @@ +package datadog.trace.instrumentation.tomcat7 + +import datadog.trace.api.Config +import spock.lang.Specification + +class ParameterCollectorImplTest extends Specification { + + void 'getContents returns empty list when no parts added'() { + expect: + new ParameterCollector.ParameterCollectorImpl(true).getContents().isEmpty() + } + + void 'addPart skips content when filename is null'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new TestPart(null, 'some body')) + + then: + collector.getContents().isEmpty() + collector.getFilenames().isEmpty() + } + + void 'addPart reads content but skips filename when filename is empty string'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new TestPart('', 'some body')) + + then: + collector.getContents() == ['some body'] + collector.getFilenames().isEmpty() + } + + void 'addPart reads content for file part with filename'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new TestPart('file.txt', 'hello world')) + + then: + collector.getContents() == ['hello world'] + collector.getFilenames() == ['file.txt'] + } + + void 'addPart reads content for multiple files'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new TestPart('a.txt', 'content-a')) + collector.addPart(new TestPart('b.txt', 'content-b')) + + then: + collector.getContents() == ['content-a', 'content-b'] + collector.getFilenames() == ['a.txt', 'b.txt'] + } + + void 'addPart truncates content at MAX_CONTENT_BYTES'() { + given: + def maxBytes = Config.get().getAppSecMaxFileContentBytes() + def collector = new ParameterCollector.ParameterCollectorImpl(true) + def longContent = 'X' * (maxBytes + 500) + + when: + collector.addPart(new TestPart('big.bin', longContent)) + + then: + collector.getContents()[0].length() == maxBytes + } + + void 'addPart reads exactly MAX_CONTENT_BYTES when content is exactly that size'() { + given: + def maxBytes = Config.get().getAppSecMaxFileContentBytes() + def collector = new ParameterCollector.ParameterCollectorImpl(true) + def content = 'Y' * maxBytes + + when: + collector.addPart(new TestPart('exact.bin', content)) + + then: + collector.getContents()[0].length() == maxBytes + } + + void 'addPart stops collecting content after MAX_FILES_TO_INSPECT files but still collects filenames'() { + given: + def maxFiles = Config.get().getAppSecMaxFileContentCount() + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + (1..maxFiles + 1).each { i -> collector.addPart(new TestPart("file${i}.txt", "content${i}")) } + + then: + collector.getContents().size() == maxFiles + collector.getFilenames().size() == maxFiles + 1 + } + + void 'addPart adds empty string when getInputStream throws'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new FailingPart('bad.txt')) + + then: + collector.getContents() == [''] + collector.getFilenames() == ['bad.txt'] + } + + void 'addPart preserves ISO-8859-1 bytes'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + def bytes = (0..255).collect { (byte) it } as byte[] + def expected = new String(bytes, 'ISO-8859-1') + + when: + collector.addPart(new RawBytesPart('binary.bin', bytes)) + + then: + collector.getContents()[0] == expected + } + + void 'addPart skips content when inspectContent is false but still collects filename'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(false) + + when: + collector.addPart(new TestPart('file.txt', 'hello world')) + + then: + collector.getContents().isEmpty() + collector.getFilenames() == ['file.txt'] + } + + void 'addPart falls back to getFilename() when getSubmittedFileName() is absent (Tomcat 7)'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + + when: + collector.addPart(new Tomcat7Part('tomcat7.txt', 'tomcat7 content')) + + then: + collector.getContents() == ['tomcat7 content'] + collector.getFilenames() == ['tomcat7.txt'] + } + + void 'addPart uses Content-Type charset declared by part'() { + given: + def collector = new ParameterCollector.ParameterCollectorImpl(true) + def text = 'héllo wörld' + def bytes = text.getBytes('UTF-8') + + when: + collector.addPart(new TestPartWithContentType('utf8.txt', bytes, 'text/plain; charset=UTF-8')) + + then: + collector.getContents()[0] == text + } + + void 'ParameterCollectorNoop getContents returns empty list'() { + expect: + ParameterCollector.ParameterCollectorNoop.INSTANCE.getContents().isEmpty() + } + + // --- helper classes --- + + static class TestPart { + private final String filename + private final String content + + TestPart(String filename, String content) { + this.filename = filename + this.content = content + } + + String getSubmittedFileName() { + filename + } + + InputStream getInputStream() { + new ByteArrayInputStream((content ?: '').getBytes('ISO-8859-1')) + } + + String getContentType() { + null + } + } + + static class FailingPart { + private final String filename + + FailingPart(String filename) { + this.filename = filename + } + + String getSubmittedFileName() { + filename + } + + InputStream getInputStream() { + throw new IOException('simulated error') + } + } + + static class RawBytesPart { + private final String filename + private final byte[] bytes + + RawBytesPart(String filename, byte[] bytes) { + this.filename = filename + this.bytes = bytes + } + + String getSubmittedFileName() { + filename + } + + InputStream getInputStream() { + new ByteArrayInputStream(bytes) + } + + String getContentType() { + 'text/plain; charset=ISO-8859-1' + } + } + + /** Simulates a Tomcat 7 ApplicationPart that only exposes getFilename(), not getSubmittedFileName(). */ + static class Tomcat7Part { + private final String filename + private final String content + + Tomcat7Part(String filename, String content) { + this.filename = filename + this.content = content + } + + String getFilename() { + filename + } + + InputStream getInputStream() { + new ByteArrayInputStream((content ?: '').getBytes('ISO-8859-1')) + } + + String getContentType() { + null + } + } + + static class TestPartWithContentType { + private final String filename + private final byte[] bytes + private final String contentType + + TestPartWithContentType(String filename, byte[] bytes, String contentType) { + this.filename = filename + this.bytes = bytes + this.contentType = contentType + } + + String getSubmittedFileName() { + filename + } + + InputStream getInputStream() { + new ByteArrayInputStream(bytes) + } + + String getContentType() { + contentType + } + } +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index fd6c384de21..e8ef80bd4a3 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -144,6 +144,8 @@ public final class ConfigDefaults { static final int DEFAULT_APPSEC_MAX_STACK_TRACE_DEPTH = 32; static final int DEFAULT_APPSEC_MAX_COLLECTED_HEADERS = 50; static final int DEFAULT_APPSEC_BODY_PARSING_SIZE_LIMIT = 10_000_000; + static final int DEFAULT_APPSEC_MAX_FILE_CONTENT_BYTES = 4096; + static final int DEFAULT_APPSEC_MAX_FILE_CONTENT_COUNT = 25; static final String DEFAULT_IAST_ENABLED = "false"; static final boolean DEFAULT_IAST_DEBUG_ENABLED = false; public static final int DEFAULT_IAST_MAX_CONCURRENT_REQUESTS = 4; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index ea1f6cb9e73..370813aa190 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -50,6 +50,8 @@ public final class AppSecConfig { public static final String APPSEC_MAX_STACK_TRACE_DEPTH = "appsec.max.stack-trace.depth"; public static final String APPSEC_MAX_STACKTRACE_DEPTH_DEPRECATED = "appsec.max.stacktrace.depth"; // old non-standard as a fallback alias + public static final String APPSEC_MAX_FILE_CONTENT_BYTES = "appsec.max.file-content.bytes"; + public static final String APPSEC_MAX_FILE_CONTENT_COUNT = "appsec.max.file-content.count"; private AppSecConfig() {} } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 34de791eee7..7e342e2cd1a 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -12,6 +12,8 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_API_SECURITY_MAX_DOWNSTREAM_REQUEST_BODY_ANALYSIS; import static datadog.trace.api.ConfigDefaults.DEFAULT_API_SECURITY_SAMPLE_DELAY; import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_BODY_PARSING_SIZE_LIMIT; +import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_MAX_FILE_CONTENT_BYTES; +import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_MAX_FILE_CONTENT_COUNT; import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_MAX_STACK_TRACES; import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_MAX_STACK_TRACE_DEPTH; import static datadog.trace.api.ConfigDefaults.DEFAULT_APPSEC_REPORTING_INBAND; @@ -215,6 +217,8 @@ import static datadog.trace.api.config.AppSecConfig.APPSEC_HTTP_BLOCKED_TEMPLATE_HTML; import static datadog.trace.api.config.AppSecConfig.APPSEC_HTTP_BLOCKED_TEMPLATE_JSON; import static datadog.trace.api.config.AppSecConfig.APPSEC_IP_ADDR_HEADER; +import static datadog.trace.api.config.AppSecConfig.APPSEC_MAX_FILE_CONTENT_BYTES; +import static datadog.trace.api.config.AppSecConfig.APPSEC_MAX_FILE_CONTENT_COUNT; import static datadog.trace.api.config.AppSecConfig.APPSEC_MAX_STACKTRACES_DEPRECATED; import static datadog.trace.api.config.AppSecConfig.APPSEC_MAX_STACKTRACE_DEPTH_DEPRECATED; import static datadog.trace.api.config.AppSecConfig.APPSEC_MAX_STACK_TRACES; @@ -1030,6 +1034,8 @@ public static String getHostName() { private final int appSecMaxStackTraces; private final int appSecMaxStackTraceDepth; private final int appSecBodyParsingSizeLimit; + private final int appSecMaxFileContentBytes; + private final int appSecMaxFileContentCount; private final boolean apiSecurityEnabled; private final float apiSecuritySampleDelay; private final int apiSecurityEndpointCollectionMessageLimit; @@ -2333,6 +2339,12 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) appSecBodyParsingSizeLimit = configProvider.getInteger( APPSEC_BODY_PARSING_SIZE_LIMIT, DEFAULT_APPSEC_BODY_PARSING_SIZE_LIMIT); + appSecMaxFileContentBytes = + configProvider.getInteger( + APPSEC_MAX_FILE_CONTENT_BYTES, DEFAULT_APPSEC_MAX_FILE_CONTENT_BYTES); + appSecMaxFileContentCount = + configProvider.getInteger( + APPSEC_MAX_FILE_CONTENT_COUNT, DEFAULT_APPSEC_MAX_FILE_CONTENT_COUNT); apiSecurityEnabled = configProvider.getBoolean( API_SECURITY_ENABLED, DEFAULT_API_SECURITY_ENABLED, API_SECURITY_ENABLED_EXPERIMENTAL); @@ -5618,6 +5630,14 @@ public int getAppSecBodyParsingSizeLimit() { return appSecBodyParsingSizeLimit; } + public int getAppSecMaxFileContentBytes() { + return appSecMaxFileContentBytes; + } + + public int getAppSecMaxFileContentCount() { + return appSecMaxFileContentCount; + } + public boolean isCloudPayloadTaggingEnabledFor(String serviceName) { return cloudPayloadTaggingServices.contains(serviceName); } diff --git a/internal-api/src/main/java/datadog/trace/api/http/MultipartContentDecoder.java b/internal-api/src/main/java/datadog/trace/api/http/MultipartContentDecoder.java index 3aefb2e2e7c..acfd45762fa 100644 --- a/internal-api/src/main/java/datadog/trace/api/http/MultipartContentDecoder.java +++ b/internal-api/src/main/java/datadog/trace/api/http/MultipartContentDecoder.java @@ -1,5 +1,7 @@ package datadog.trace.api.http; +import java.io.IOException; +import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; @@ -8,6 +10,17 @@ /** Decodes multipart file content bytes to String using the per-part Content-Type charset. */ public final class MultipartContentDecoder { + public static String readInputStream(InputStream is, int maxBytes, String contentType) + throws IOException { + byte[] buf = new byte[maxBytes]; + int total = 0; + int n; + while (total < maxBytes && (n = is.read(buf, total, maxBytes - total)) != -1) { + total += n; + } + return decodeBytes(buf, total, contentType); + } + public static String decodeBytes(byte[] buf, int length, String contentType) { Charset charset = extractCharset(contentType); if (charset == null) charset = Charset.defaultCharset(); diff --git a/internal-api/src/test/java/datadog/trace/api/http/MultipartContentDecoderTest.java b/internal-api/src/test/java/datadog/trace/api/http/MultipartContentDecoderTest.java index de028edf24d..013ac39b3c7 100644 --- a/internal-api/src/test/java/datadog/trace/api/http/MultipartContentDecoderTest.java +++ b/internal-api/src/test/java/datadog/trace/api/http/MultipartContentDecoderTest.java @@ -3,6 +3,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -148,4 +151,25 @@ void decodeBytesUsesQuotedDeclaredCharset() { MultipartContentDecoder.decodeBytes( bytes, bytes.length, "text/plain; charset=\"ISO-8859-1\"")); } + + @Test + void readInputStreamTruncatesAtMaxBytes() throws IOException { + byte[] data = "hello world".getBytes(StandardCharsets.UTF_8); + assertEquals( + "hello", MultipartContentDecoder.readInputStream(new ByteArrayInputStream(data), 5, null)); + } + + @Test + void readInputStreamHandlesMultipleReadCallsToFillBuffer() throws IOException { + byte[] data = "hello world".getBytes(StandardCharsets.UTF_8); + // InputStream that returns 2 bytes per read() call to exercise the accumulation loop. + InputStream slow = + new ByteArrayInputStream(data) { + @Override + public synchronized int read(byte[] b, int off, int len) { + return super.read(b, off, Math.min(len, 2)); + } + }; + assertEquals("hello world", MultipartContentDecoder.readInputStream(slow, data.length, null)); + } } diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index a1569256c66..e1ad02e47bc 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -289,6 +289,22 @@ "aliases": [] } ], + "DD_APPSEC_MAX_FILE_CONTENT_BYTES": [ + { + "version": "A", + "type": "int", + "default": "4096", + "aliases": [] + } + ], + "DD_APPSEC_MAX_FILE_CONTENT_COUNT": [ + { + "version": "A", + "type": "int", + "default": "25", + "aliases": [] + } + ], "DD_APPSEC_MAX_STACKTRACES": [ { "version": "A",