Skip to content

Commit

Permalink
IAST support for commons fileupload (#6089)
Browse files Browse the repository at this point in the history
  • Loading branch information
DDJavierSantos authored Nov 16, 2023
1 parent 74313a3 commit df6decb
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dd-java-agent/agent-jmxfetch/integrations-core
13 changes: 13 additions & 0 deletions dd-java-agent/instrumentation/commons-fileupload/build.gradle
Original file line number Diff line number Diff line change
@@ -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: '+'
}
Original file line number Diff line number Diff line change
@@ -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<String> 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<String, String> onExit(@Advice.Return final Map<String, String> map) {
if (!map.isEmpty()) {
final PropagationModule module = InstrumentationBridge.PROPAGATION;
if (module != null) {
final IastContext ctx = IastContext.Provider.get();
for (final Map.Entry<String, String> entry : map.entrySet()) {
if (entry.getValue() != null) {
module.taint(
ctx, entry.getValue(), SourceTypes.REQUEST_MULTIPART_PARAMETER, entry.getKey());
}
}
}
}
return map;
}
}
}
Original file line number Diff line number Diff line change
@@ -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 | _
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit df6decb

Please sign in to comment.