diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bf87304df15..b8e6662f7c7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -37,6 +37,9 @@ dd-smoke-tests/debugger-integration-tests/ @DataDog/debugger-java dd-java-agent/agent-iast/ @DataDog/asm-java dd-java-agent/instrumentation/*iast* @DataDog/asm-java dd-java-agent/instrumentation/*appsec* @DataDog/asm-java +dd-java-agent/instrumentation/json/ @DataDog/asm-java +dd-smoke-tests/iast-util/ @DataDog/asm-java +dd-java-agent/instrumentation/commons-fileupload/ @DataDog/asm-java **/appsec/ @DataDog/asm-java **/iast/ @DataDog/asm-java **/Iast*.java @DataDog/asm-java diff --git a/dd-java-agent/agent-jmxfetch/integrations-core b/dd-java-agent/agent-jmxfetch/integrations-core index d211ef6ec4c..03aed80d105 160000 --- a/dd-java-agent/agent-jmxfetch/integrations-core +++ b/dd-java-agent/agent-jmxfetch/integrations-core @@ -1 +1 @@ -Subproject commit d211ef6ec4c9584df4b9f430dd10382f169d4727 +Subproject commit 03aed80d105aa81b047e74c6da086165cac5ff6f diff --git a/dd-java-agent/instrumentation/commons-fileupload/build.gradle b/dd-java-agent/instrumentation/commons-fileupload/build.gradle new file mode 100644 index 00000000000..eadd96edbea --- /dev/null +++ b/dd-java-agent/instrumentation/commons-fileupload/build.gradle @@ -0,0 +1,13 @@ + +apply from: "$rootDir/gradle/java.gradle" +addTestSuiteForDir('latestDepTest', 'test') + +dependencies { + compileOnly group: 'org.apache.commons', name: 'commons-fileupload2', version: '2.0.0-M1' + testImplementation group: 'org.apache.commons', name: 'commons-fileupload2', version: '2.0.0-M1' + testImplementation group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '7.0.0' + + + testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') + latestDepTestImplementation group: 'org.apache.commons', name: 'commons-fileupload2', version: '+' +} diff --git a/dd-java-agent/instrumentation/commons-fileupload/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileuploadInstrumenter.java b/dd-java-agent/instrumentation/commons-fileupload/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileuploadInstrumenter.java new file mode 100644 index 00000000000..f463330067f --- /dev/null +++ b/dd-java-agent/instrumentation/commons-fileupload/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileuploadInstrumenter.java @@ -0,0 +1,68 @@ +package datadog.trace.instrumentation.commons.fileupload; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public class CommonsFileuploadInstrumenter extends Instrumenter.Iast + implements Instrumenter.ForConfiguredTypes { + + public CommonsFileuploadInstrumenter() { + super("commons-fileupload"); + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + isMethod() + .and(named("parse")) + .and(isPublic()) + .and(returns(Map.class)) + .and(takesArguments(char[].class, int.class, int.class, char.class)), + getClass().getName() + "$ParseAdvice"); + } + + @Override + public Collection configuredMatchingTypes() { + return Arrays.asList( + new String[] { + "org.apache.commons.fileupload.ParameterParser", + "org.apache.tomcat.util.http.fileupload.ParameterParser" + }); + } + + public static class ParseAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_MULTIPART_PARAMETER) + public static Map onExit(@Advice.Return final Map map) { + if (!map.isEmpty()) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + final IastContext ctx = IastContext.Provider.get(); + for (final Map.Entry entry : map.entrySet()) { + if (entry.getValue() != null) { + module.taint( + ctx, entry.getValue(), SourceTypes.REQUEST_MULTIPART_PARAMETER, entry.getKey()); + } + } + } + } + return map; + } + } +} diff --git a/dd-java-agent/instrumentation/commons-fileupload/src/test/groovy/MultipartInstrumentationTest.groovy b/dd-java-agent/instrumentation/commons-fileupload/src/test/groovy/MultipartInstrumentationTest.groovy new file mode 100644 index 00000000000..4e780a02d1a --- /dev/null +++ b/dd-java-agent/instrumentation/commons-fileupload/src/test/groovy/MultipartInstrumentationTest.groovy @@ -0,0 +1,40 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.SourceTypes +import datadog.trace.api.iast.propagation.PropagationModule + + +class MultipartInstrumentationTest extends AgentTestRunner { + @Override + protected void configurePreAgent() { + injectSysConfig('dd.iast.enabled', 'true') + } + + @Override + void cleanup() { + InstrumentationBridge.clearIastModules() + } + + void 'test commons fileupload2 ParameterParser.parse'() { + given: + final module = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(module) + final content = "Content-Disposition: form-data; name=\"file\"; filename=\"=?ISO-8859-1?B?SWYgeW91IGNhbiByZWFkIHRoaXMgeW8=?= =?ISO-8859-2?B?dSB1bmRlcnN0YW5kIHRoZSBleGFtcGxlLg==?=\"\r\n" + final parser = clazz.newInstance() + + when: + parser.parse(content, new char[]{ + ',', ';' + }) + + then: + 1 * module.taint(null, 'file', SourceTypes.REQUEST_MULTIPART_PARAMETER, 'name') + 1 * module.taint(null, _, SourceTypes.REQUEST_MULTIPART_PARAMETER, 'filename') + 0 * _ + + where: + clazz | _ + org.apache.commons.fileupload.ParameterParser | _ + org.apache.tomcat.util.http.fileupload.ParameterParser | _ + } +} diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy index 4217258e570..ee0cc033ecc 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy @@ -111,6 +111,34 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { } + void 'Multipart Request original file name'(){ + given: + String url = "http://localhost:${httpPort}/multipart" + + RequestBody requestBody = new MultipartBody.Builder().setType(MultipartBody.FORM) + .addFormDataPart("theFile", "theFileName", + RequestBody.create(MediaType.parse("text/plain"), "FILE_CONTENT")) + .addFormDataPart("param1", "param1Value") + .build() + + Request request = new Request.Builder() + .url(url) + .post(requestBody) + .build() + when: + final retValue = client.newCall(request).execute().body().string() + + then: + retValue == "fileName: theFile" + hasTainted { tainted -> + tainted.value == 'theFileName' && + tainted.ranges[0].source.name == 'filename' && + tainted.ranges[0].source.origin == 'http.request.multipart.parameter' + } + + } + + void 'iast.enabled tag is present'() { setup: String url = "http://localhost:${httpPort}/greeting" diff --git a/settings.gradle b/settings.gradle index e938fb743ae..5b7b2450bf1 100644 --- a/settings.gradle +++ b/settings.gradle @@ -175,6 +175,7 @@ include ':dd-java-agent:instrumentation:classloading:jsr14-testing' include ':dd-java-agent:instrumentation:classloading:osgi-testing' include ':dd-java-agent:instrumentation:classloading:tomcat-testing' include ':dd-java-agent:instrumentation:commons-codec-1' +include ':dd-java-agent:instrumentation:commons-fileupload' include ':dd-java-agent:instrumentation:commons-httpclient-2' include ':dd-java-agent:instrumentation:commons-lang-2' include ':dd-java-agent:instrumentation:commons-lang-3'