Skip to content

Commit 37e7d09

Browse files
committed
feat(appsec): expose uploaded file content as WAF address server.request.body.files_content
Introduces a new AppSec WAF address `server.request.body.files_content` (`List<String>`) that exposes the content of each uploaded file in a multipart/form-data request. Entries correspond positionally to the existing `server.request.body.filenames` address. Content is capped at 4 096 bytes per file (ISO-8859-1) to keep memory usage bounded. Changes: - KnownAddresses: add REQUEST_FILES_CONTENT + forName() case - Events: add requestFilesContent event (ID 31); FILE_WRITTEN bumped to 32 - InstrumentationGateway: register the new BiFunction case - GatewayBridge: add onRequestFilesContent handler + DATA_DEPENDENCIES entry - CommonsFileUploadAppSecModule: after firing filenames, fire content (skipped when the filenames event already blocked the request) - Unit tests: GatewayBridgeSpecification, GatewayBridgeIGRegistrationSpecification, KnownAddressesSpecificationForkedTest - Smoke test: 'block request based on malicious file upload content' verifies end-to-end blocking via a custom WAF rule on the new address Closes APPSEC-61875
1 parent 42f154d commit 37e7d09

File tree

9 files changed

+221
-13
lines changed

9 files changed

+221
-13
lines changed

dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ public interface KnownAddresses {
7474
Address<Long> REQUEST_COMBINED_FILE_SIZE =
7575
new Address<>("server.request.body.combined_file_size");
7676

77+
/**
78+
* Contains the content of each uploaded file in a multipart/form-data request. Each entry in the
79+
* list corresponds positionally to {@link #REQUEST_FILES_FILENAMES}. Content is truncated to a
80+
* maximum size to avoid excessive memory usage. Available only on inspected multipart/form-data
81+
* requests.
82+
*/
83+
Address<List<String>> REQUEST_FILES_CONTENT = new Address<>("server.request.body.files_content");
84+
7785
/**
7886
* The parsed query string.
7987
*
@@ -205,6 +213,8 @@ static Address<?> forName(String name) {
205213
return REQUEST_FILES_FILENAMES;
206214
case "server.request.body.combined_file_size":
207215
return REQUEST_COMBINED_FILE_SIZE;
216+
case "server.request.body.files_content":
217+
return REQUEST_FILES_CONTENT;
208218
case "server.request.query":
209219
return REQUEST_QUERY;
210220
case "server.request.headers.no_cookies":

dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ public class GatewayBridge {
131131
private volatile DataSubscriberInfo execCmdSubInfo;
132132
private volatile DataSubscriberInfo shellCmdSubInfo;
133133
private volatile DataSubscriberInfo requestFilesFilenamesSubInfo;
134+
private volatile DataSubscriberInfo requestFilesContentSubInfo;
134135

135136
public GatewayBridge(
136137
SubscriptionService subscriptionService,
@@ -208,6 +209,10 @@ public void init() {
208209
subscriptionService.registerCallback(
209210
EVENTS.requestFilesFilenames(), this::onRequestFilesFilenames);
210211
}
212+
if (additionalIGEvents.contains(EVENTS.requestFilesContent())) {
213+
subscriptionService.registerCallback(
214+
EVENTS.requestFilesContent(), this::onRequestFilesContent);
215+
}
211216
}
212217

213218
/**
@@ -235,6 +240,7 @@ public void reset() {
235240
execCmdSubInfo = null;
236241
shellCmdSubInfo = null;
237242
requestFilesFilenamesSubInfo = null;
243+
requestFilesContentSubInfo = null;
238244
}
239245

240246
private Flow<Void> onUser(final RequestContext ctx_, final String user) {
@@ -605,6 +611,31 @@ private Flow<Void> onRequestFilesFilenames(RequestContext ctx_, List<String> fil
605611
}
606612
}
607613

614+
private Flow<Void> onRequestFilesContent(RequestContext ctx_, List<String> filesContent) {
615+
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
616+
if (ctx == null || filesContent == null || filesContent.isEmpty()) {
617+
return NoopFlow.INSTANCE;
618+
}
619+
while (true) {
620+
DataSubscriberInfo subInfo = requestFilesContentSubInfo;
621+
if (subInfo == null) {
622+
subInfo = producerService.getDataSubscribers(KnownAddresses.REQUEST_FILES_CONTENT);
623+
requestFilesContentSubInfo = subInfo;
624+
}
625+
if (subInfo == null || subInfo.isEmpty()) {
626+
return NoopFlow.INSTANCE;
627+
}
628+
DataBundle bundle =
629+
new SingletonDataBundle<>(KnownAddresses.REQUEST_FILES_CONTENT, filesContent);
630+
try {
631+
GatewayContext gwCtx = new GatewayContext(false);
632+
return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx);
633+
} catch (ExpiredSubscriberInfoException e) {
634+
requestFilesContentSubInfo = null;
635+
}
636+
}
637+
}
638+
608639
private Flow<Void> onDatabaseSqlQuery(RequestContext ctx_, String sql) {
609640
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
610641
if (ctx == null) {
@@ -1464,6 +1495,7 @@ private static class IGAppSecEventDependencies {
14641495
DATA_DEPENDENCIES.put(KnownAddresses.REQUEST_BODY_OBJECT, l(EVENTS.requestBodyProcessed()));
14651496
DATA_DEPENDENCIES.put(
14661497
KnownAddresses.REQUEST_FILES_FILENAMES, l(EVENTS.requestFilesFilenames()));
1498+
DATA_DEPENDENCIES.put(KnownAddresses.REQUEST_FILES_CONTENT, l(EVENTS.requestFilesContent()));
14671499
}
14681500

14691501
private static Collection<datadog.trace.api.gateway.EventType<?>> l(

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecificationForkedTest.groovy

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class KnownAddressesSpecificationForkedTest extends Specification {
2626
'server.request.body.files_field_names',
2727
'server.request.body.filenames',
2828
'server.request.body.combined_file_size',
29+
'server.request.body.files_content',
2930
'server.request.query',
3031
'server.request.headers.no_cookies',
3132
'grpc.server.method',
@@ -58,7 +59,7 @@ class KnownAddressesSpecificationForkedTest extends Specification {
5859

5960
void 'number of known addresses is expected number'() {
6061
expect:
61-
Address.instanceCount() == 46
62+
Address.instanceCount() == 47
6263
KnownAddresses.WAF_CONTEXT_PROCESSOR.serial == Address.instanceCount() - 1
6364
}
6465
}

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeIGRegistrationSpecification.groovy

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,15 @@ class GatewayBridgeIGRegistrationSpecification extends DDSpecification {
3434
then:
3535
1 * ig.registerCallback(Events.REQUEST_BODY_DONE, _)
3636
}
37+
38+
void 'requestFilesContent is registered via data address'() {
39+
given:
40+
1 * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_FILES_CONTENT]
41+
42+
when:
43+
bridge.init()
44+
45+
then:
46+
1 * ig.registerCallback(Events.get().requestFilesContent(), _)
47+
}
3748
}

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class GatewayBridgeSpecification extends DDSpecification {
123123
BiFunction<RequestContext, String, Flow<Void>> fileLoadedCB
124124
BiFunction<RequestContext, String, Flow<Void>> fileWrittenCB
125125
BiFunction<RequestContext, List<String>, Flow<Void>> requestFilesFilenamesCB
126+
BiFunction<RequestContext, List<String>, Flow<Void>> requestFilesContentCB
126127
BiFunction<RequestContext, String, Flow<Void>> requestSessionCB
127128
BiFunction<RequestContext, String[], Flow<Void>> execCmdCB
128129
BiFunction<RequestContext, String, Flow<Void>> shellCmdCB
@@ -463,7 +464,7 @@ class GatewayBridgeSpecification extends DDSpecification {
463464

464465
void callInitAndCaptureCBs() {
465466
// force all callbacks to be registered
466-
_ * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_PATH_PARAMS, KnownAddresses.REQUEST_BODY_OBJECT, KnownAddresses.REQUEST_FILES_FILENAMES]
467+
_ * eventDispatcher.allSubscribedDataAddresses() >> [KnownAddresses.REQUEST_PATH_PARAMS, KnownAddresses.REQUEST_BODY_OBJECT, KnownAddresses.REQUEST_FILES_FILENAMES, KnownAddresses.REQUEST_FILES_CONTENT]
467468

468469
1 * ig.registerCallback(EVENTS.requestStarted(), _) >> {
469470
requestStartedCB = it[1]; null
@@ -561,6 +562,9 @@ class GatewayBridgeSpecification extends DDSpecification {
561562
1 * ig.registerCallback(EVENTS.requestFilesFilenames(), _) >> {
562563
requestFilesFilenamesCB = it[1]; null
563564
}
565+
1 * ig.registerCallback(EVENTS.requestFilesContent(), _) >> {
566+
requestFilesContentCB = it[1]; null
567+
}
564568
0 * ig.registerCallback(_, _)
565569

566570
bridge.init()
@@ -1142,6 +1146,38 @@ class GatewayBridgeSpecification extends DDSpecification {
11421146
0 * eventDispatcher.publishDataEvent(*_)
11431147
}
11441148

1149+
void 'process request files content'() {
1150+
setup:
1151+
final filesContent = ['%PDF-1.4 malicious content', '#!/bin/bash\nrm -rf /']
1152+
eventDispatcher.getDataSubscribers({
1153+
KnownAddresses.REQUEST_FILES_CONTENT in it
1154+
}) >> nonEmptyDsInfo
1155+
DataBundle bundle
1156+
GatewayContext gatewayContext
1157+
1158+
when:
1159+
Flow<?> flow = requestFilesContentCB.apply(ctx, filesContent)
1160+
1161+
then:
1162+
1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> {
1163+
a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE
1164+
}
1165+
bundle.get(KnownAddresses.REQUEST_FILES_CONTENT) == filesContent
1166+
flow.result == null
1167+
flow.action == Flow.Action.Noop.INSTANCE
1168+
gatewayContext.isTransient == false
1169+
gatewayContext.isRasp == false
1170+
}
1171+
1172+
void 'process request files content with empty list returns noop'() {
1173+
when:
1174+
Flow<?> flow = requestFilesContentCB.apply(ctx, [])
1175+
1176+
then:
1177+
flow == NoopFlow.INSTANCE
1178+
0 * eventDispatcher.publishDataEvent(*_)
1179+
}
1180+
11451181
void 'process exec cmd'() {
11461182
setup:
11471183
final cmd = ['/bin/../usr/bin/reboot', '-f'] as String[]

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 --"

internal-api/src/main/java/datadog/trace/api/gateway/Events.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,19 @@ public EventType<BiFunction<RequestContext, List<String>, Flow<Void>>> requestFi
396396
REQUEST_FILES_FILENAMES;
397397
}
398398

399-
static final int FILE_WRITTEN_ID = 31;
399+
static final int REQUEST_FILES_CONTENT_ID = 31;
400+
401+
@SuppressWarnings("rawtypes")
402+
private static final EventType REQUEST_FILES_CONTENT =
403+
new ET<>("request.body.files.content", REQUEST_FILES_CONTENT_ID);
404+
405+
/** Contents of files uploaded in a multipart/form-data request */
406+
@SuppressWarnings("unchecked")
407+
public EventType<BiFunction<RequestContext, List<String>, Flow<Void>>> requestFilesContent() {
408+
return (EventType<BiFunction<RequestContext, List<String>, Flow<Void>>>) REQUEST_FILES_CONTENT;
409+
}
410+
411+
static final int FILE_WRITTEN_ID = 32;
400412

401413
@SuppressWarnings("rawtypes")
402414
private static final EventType FILE_WRITTEN = new ET<>("file.written", FILE_WRITTEN_ID);

internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static datadog.trace.api.gateway.Events.REQUEST_BODY_START_ID;
2020
import static datadog.trace.api.gateway.Events.REQUEST_CLIENT_SOCKET_ADDRESS_ID;
2121
import static datadog.trace.api.gateway.Events.REQUEST_ENDED_ID;
22+
import static datadog.trace.api.gateway.Events.REQUEST_FILES_CONTENT_ID;
2223
import static datadog.trace.api.gateway.Events.REQUEST_FILES_FILENAMES_ID;
2324
import static datadog.trace.api.gateway.Events.REQUEST_HEADER_DONE_ID;
2425
import static datadog.trace.api.gateway.Events.REQUEST_HEADER_ID;
@@ -363,6 +364,7 @@ public Flow<Void> apply(RequestContext ctx, StoredBodySupplier storedBodySupplie
363364
case REQUEST_BODY_CONVERTED_ID:
364365
case RESPONSE_BODY_ID:
365366
case REQUEST_FILES_FILENAMES_ID:
367+
case REQUEST_FILES_CONTENT_ID:
366368
return (C)
367369
new BiFunction<RequestContext, Object, Flow<Void>>() {
368370
@Override

0 commit comments

Comments
 (0)