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..c04c4d7b6f0 --- /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,36 @@ +package datadog.trace.instrumentation.jersey2; + +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 { + + private MultiPartHelper() {} + + public static void collectBodyPart( + FormDataBodyPart bodyPart, Map> bodyMap, List filenames) { + 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()); + } + if (filenames != null) { + String filename = filenameFromBodyPart(bodyPart); + if (filename != null) { + filenames.add(filename); + } + } + } + + public static String filenameFromBodyPart(FormDataBodyPart bodyPart) { + FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition(); + if (cd == null) return null; + String filename = cd.getFileName(); + return (filename == null || filename.isEmpty()) ? null : filename; + } +} 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..eeb3eb3756c 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 @@ -23,12 +23,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 +46,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 +75,49 @@ 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()); + if (callback == null && filenamesCallback == null) { return; } - Map> map = new HashMap<>(); + Map> map = callback != null ? new HashMap<>() : null; + List filenames = filenamesCallback != 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); + } - 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); + 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 MultiPartReaderServerSide/readMultiPart)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } - - values.add(v); } - 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 (filenames != null && !filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + Flow.Action filenamesAction = filenamesFlow.getAction(); + if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) filenamesAction; + BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); + if (blockResponseFunction != null) { + blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } 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..ef6ac646541 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-2.0/src/test/groovy/MultiPartHelperTest.groovy @@ -0,0 +1,159 @@ +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 { + + // filenameFromBodyPart + + def "returns null when content disposition is null"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> null + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == null + } + + def "returns null when filename is null or empty"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> rawFilename + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == null + + where: + rawFilename << [null, ''] + } + + def "extracts filename"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> filename + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == filename + + where: + filename << ['report.php', 'upload.txt', 'shell;evil.php', 'file"name.php'] + } + + // 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) + + 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) + + 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) + } + + 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) + MultiPartHelper.collectBodyPart(bodyPart, map, 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) + + 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) + } + + 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) + + then: + map == [file: ['content']] + filenames == ['upload.txt'] + } +} 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..611b384d86c --- /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,36 @@ +package datadog.trace.instrumentation.jersey3; + +import jakarta.ws.rs.core.MediaType; +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 { + + private MultiPartHelper() {} + + public static void collectBodyPart( + FormDataBodyPart bodyPart, Map> bodyMap, List filenames) { + 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()); + } + if (filenames != null) { + String filename = filenameFromBodyPart(bodyPart); + if (filename != null) { + filenames.add(filename); + } + } + } + + public static String filenameFromBodyPart(FormDataBodyPart bodyPart) { + FormDataContentDisposition cd = bodyPart.getFormDataContentDisposition(); + if (cd == null) return null; + String filename = cd.getFileName(); + return (filename == null || filename.isEmpty()) ? null : filename; + } +} 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..6bbf9fc7ea2 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 @@ -18,7 +18,6 @@ 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 +27,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 +46,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 +75,49 @@ 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()); + if (callback == null && filenamesCallback == null) { return; } - Map> map = new HashMap<>(); + Map> map = callback != null ? new HashMap<>() : null; + List filenames = filenamesCallback != 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); + } - 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); + 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 MultiPartReaderServerSide/readMultiPart)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } - - values.add(v); } - 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 (filenames != null && !filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + Flow.Action filenamesAction = filenamesFlow.getAction(); + if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) filenamesAction; + BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); + if (blockResponseFunction != null) { + blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } } } } 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..2053f413578 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/jersey-appsec/jersey-appsec-3.0/src/test/groovy/MultiPartHelperTest.groovy @@ -0,0 +1,159 @@ +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 { + + // filenameFromBodyPart + + def "returns null when content disposition is null"() { + given: + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> null + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == null + } + + def "returns null when filename is null or empty"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> rawFilename + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == null + + where: + rawFilename << [null, ''] + } + + def "extracts filename"() { + given: + def cd = Mock(FormDataContentDisposition) + cd.getFileName() >> filename + def bodyPart = Mock(FormDataBodyPart) + bodyPart.getFormDataContentDisposition() >> cd + + expect: + MultiPartHelper.filenameFromBodyPart(bodyPart) == filename + + where: + filename << ['report.php', 'upload.txt', 'shell;evil.php', 'file"name.php'] + } + + // 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) + + 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) + + 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) + } + + 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) + MultiPartHelper.collectBodyPart(bodyPart, map, 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) + + 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) + } + + 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) + + then: + map == [file: ['content']] + filenames == ['upload.txt'] + } +} 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..6e49ceaaa90 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 @@ -45,6 +45,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 +77,50 @@ 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()); + if (callback == null && filenamesCallback == 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); + 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(); + } } } - 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 (filenamesCallback != null) { + List filenames = MultipartHelper.collectFilenames(ret); + if (!filenames.isEmpty()) { + Flow filenamesFlow = filenamesCallback.apply(reqCtx, filenames); + Flow.Action filenamesAction = filenamesFlow.getAction(); + if (t == null && filenamesAction instanceof Flow.Action.RequestBlockingAction) { + Flow.Action.RequestBlockingAction rba = + (Flow.Action.RequestBlockingAction) filenamesAction; + BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction(); + if (blockResponseFunction != null) { + blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba); + t = new BlockingException("Blocked request (multipart file upload)"); + reqCtx.getTraceSegment().effectivelyBlocked(); + } + } } } } 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..b14b55d60ea --- /dev/null +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/main/java/datadog/trace/instrumentation/resteasy/MultipartHelper.java @@ -0,0 +1,105 @@ +package datadog.trace.instrumentation.resteasy; + +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 { + + 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; + } + + // 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) { + 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 null; + 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++)); + } + String name = sb.toString(); + return name.isEmpty() ? null : name; + } else { + int start = i; + while (i < len && cd.charAt(i) != ';') i++; + String name = cd.substring(start, i).trim(); + return name.isEmpty() ? null : name; + } + } + } + } + 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..fea89c7a5ba --- /dev/null +++ b/dd-java-agent/instrumentation/resteasy/resteasy-appsec-3.0/src/test/groovy/MultipartHelperTest.groovy @@ -0,0 +1,103 @@ +import datadog.trace.instrumentation.resteasy.MultipartHelper +import spock.lang.Specification + +class MultipartHelperTest extends Specification { + + 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 + } +}