Skip to content

Commit 0de9320

Browse files
committed
feat(appsec): add file content event to CommonsFileUpload hook and smoke test
Extend CommonsFileUploadAppSecModule to fire the new server.request.body.files_content WAF address after the filenames event (skipped when the request is already being blocked), and add a smoke test that verifies a WAF rule on that address blocks a request whose uploaded file has a safe name but malicious content.
1 parent 33212a8 commit 0de9320

File tree

2 files changed

+114
-10
lines changed

2 files changed

+114
-10
lines changed

dd-java-agent/instrumentation/commons-fileupload-1.5/src/main/java/datadog/trace/instrumentation/commons/fileupload/CommonsFileUploadAppSecModule.java

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
import datadog.trace.api.gateway.RequestContext;
1818
import datadog.trace.api.gateway.RequestContextSlot;
1919
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
20+
import java.io.IOException;
21+
import java.io.InputStream;
22+
import java.nio.charset.StandardCharsets;
2023
import java.util.ArrayList;
2124
import java.util.List;
2225
import java.util.function.BiFunction;
@@ -47,6 +50,9 @@ public void methodAdvice(MethodTransformer transformer) {
4750

4851
@RequiresRequestContext(RequestContextSlot.APPSEC)
4952
public static class ParseRequestAdvice {
53+
54+
static final int MAX_FILE_CONTENT_BYTES = 4096;
55+
5056
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
5157
static void after(
5258
@Advice.Return final List<FileItem> fileItems,
@@ -57,11 +63,6 @@ static void after(
5763
}
5864

5965
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
60-
BiFunction<RequestContext, List<String>, Flow<Void>> callback =
61-
cbp.getCallback(EVENTS.requestFilesFilenames());
62-
if (callback == null) {
63-
return;
64-
}
6566

6667
List<String> filenames = new ArrayList<>();
6768
for (FileItem fileItem : fileItems) {
@@ -77,14 +78,65 @@ static void after(
7778
return;
7879
}
7980

80-
Flow<Void> flow = callback.apply(reqCtx, filenames);
81-
Flow.Action action = flow.getAction();
82-
if (action instanceof Flow.Action.RequestBlockingAction) {
83-
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
81+
// Fire filenames event
82+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCallback =
83+
cbp.getCallback(EVENTS.requestFilesFilenames());
84+
if (filenamesCallback != null) {
85+
Flow<Void> flow = filenamesCallback.apply(reqCtx, filenames);
86+
Flow.Action action = flow.getAction();
87+
if (action instanceof Flow.Action.RequestBlockingAction) {
88+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
89+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
90+
if (brf != null) {
91+
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
92+
t = new BlockingException("Blocked request (multipart file upload)");
93+
reqCtx.getTraceSegment().effectivelyBlocked();
94+
return;
95+
}
96+
}
97+
}
98+
99+
// Fire content event only if not blocked
100+
BiFunction<RequestContext, List<String>, Flow<Void>> contentCallback =
101+
cbp.getCallback(EVENTS.requestFilesContent());
102+
if (contentCallback == null) {
103+
return;
104+
}
105+
List<String> filesContent = new ArrayList<>();
106+
for (FileItem fileItem : fileItems) {
107+
if (fileItem.isFormField()) {
108+
continue;
109+
}
110+
String name = fileItem.getName();
111+
if (name == null || name.isEmpty()) {
112+
continue;
113+
}
114+
String content = "";
115+
try {
116+
InputStream is = fileItem.getInputStream();
117+
byte[] buf = new byte[MAX_FILE_CONTENT_BYTES];
118+
int total = 0;
119+
int n;
120+
while (total < MAX_FILE_CONTENT_BYTES
121+
&& (n = is.read(buf, total, MAX_FILE_CONTENT_BYTES - total)) != -1) {
122+
total += n;
123+
}
124+
content = new String(buf, 0, total, StandardCharsets.ISO_8859_1);
125+
} catch (IOException ignored) {
126+
}
127+
filesContent.add(content);
128+
}
129+
if (filesContent.isEmpty()) {
130+
return;
131+
}
132+
Flow<Void> contentFlow = contentCallback.apply(reqCtx, filesContent);
133+
Flow.Action contentAction = contentFlow.getAction();
134+
if (contentAction instanceof Flow.Action.RequestBlockingAction) {
135+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) contentAction;
84136
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
85137
if (brf != null) {
86138
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
87-
t = new BlockingException("Blocked request (multipart file upload)");
139+
t = new BlockingException("Blocked request (multipart file upload content)");
88140
reqCtx.getTraceSegment().effectivelyBlocked();
89141
}
90142
}

dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,26 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest {
230230
transformers: [],
231231
on_match : ['block']
232232
],
233+
[
234+
id : '__test_file_upload_content_block',
235+
name : 'test rule to block on malicious file upload content',
236+
tags : [
237+
type : 'unrestricted-file-upload',
238+
category : 'attack_attempt',
239+
confidence: '1',
240+
],
241+
conditions : [
242+
[
243+
parameters: [
244+
inputs: [[address: 'server.request.body.files_content']],
245+
regex : 'dd-test-malicious-file-content',
246+
],
247+
operator : 'match_regex',
248+
]
249+
],
250+
transformers: [],
251+
on_match : ['block']
252+
],
233253
[
234254
id : "apiA-100-001",
235255
name: "API 10 tag rule on request headers",
@@ -611,6 +631,38 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest {
611631
}
612632
}
613633

634+
void 'block request based on malicious file upload content'() {
635+
when:
636+
String url = "http://localhost:${httpPort}/upload"
637+
def requestBody = new okhttp3.MultipartBody.Builder()
638+
.setType(okhttp3.MultipartBody.FORM)
639+
.addFormDataPart('file', 'safe-document.txt',
640+
RequestBody.create(MediaType.parse('application/octet-stream'), 'dd-test-malicious-file-content'))
641+
.build()
642+
def request = new Request.Builder()
643+
.url(url)
644+
.post(requestBody)
645+
.build()
646+
def response = client.newCall(request).execute()
647+
def responseBodyStr = response.body().string()
648+
649+
then:
650+
responseBodyStr.contains("blocked")
651+
response.code() == 403
652+
653+
when:
654+
waitForTraceCount(1) == 1
655+
656+
then:
657+
rootSpans.size() == 1
658+
forEachRootSpanTrigger {
659+
assert it['rule']['id'] == '__test_file_upload_content_block'
660+
}
661+
rootSpans.each {
662+
assert it.meta.get('appsec.blocked') != null, 'appsec.blocked is not set'
663+
}
664+
}
665+
614666
void 'rasp reports stacktrace on sql injection'() {
615667
when:
616668
String url = "http://localhost:${httpPort}/sqli/query?id=' OR 1=1 --"

0 commit comments

Comments
 (0)