Skip to content

Commit 0f42e0a

Browse files
authored
Add String length truncation limit to ObjectIntrospector and update truncation metrics (#8825)
What Does This Do Add a new truncation limit to strings in the ObjectIntrospector (This is the same limit that we apply in the Libddwaf) Add three flags to the State to know if there is object depth, collection size o String length truncation Before return the Object check the flags and trigger the metrics if it's necessary Motivation WAF truncation metrics were implemented attending the information of truncation done in the libddwaf but in the cases that the object send to the WAF is transformed using ObjectIntrospector there is a truncation that we are not reporting via metrics
1 parent b8e7758 commit 0f42e0a

File tree

3 files changed

+201
-54
lines changed

3 files changed

+201
-54
lines changed

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

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.datadog.appsec.event.data;
22

3+
import com.datadog.appsec.gateway.AppSecRequestContext;
34
import datadog.trace.api.Platform;
5+
import datadog.trace.api.telemetry.WafMetricCollector;
46
import java.lang.reflect.Array;
57
import java.lang.reflect.Field;
68
import java.lang.reflect.InvocationTargetException;
@@ -16,6 +18,7 @@
1618
public final class ObjectIntrospection {
1719
private static final int MAX_DEPTH = 20;
1820
private static final int MAX_ELEMENTS = 256;
21+
private static final int MAX_STRING_LENGTH = 4096;
1922
private static final Logger log = LoggerFactory.getLogger(ObjectIntrospection.class);
2023

2124
private static final Method trySetAccessible;
@@ -60,20 +63,32 @@ private ObjectIntrospection() {}
6063
* <p>Certain instance fields are excluded. Right now, this includes metaClass fields in Groovy
6164
* objects and this$0 fields in inner classes.
6265
*
63-
* <p>Only string values are preserved. Numbers or booleans are removed, since we do not expect
64-
* rules to detect malicious payloads in these types. An exception to this are map keys, which are
65-
* always converted to strings.
66-
*
6766
* @param obj an arbitrary object
67+
* @param requestContext the request context
6868
* @return the converted object
6969
*/
70-
public static Object convert(Object obj) {
71-
return guardedConversion(obj, 0, new State());
70+
public static Object convert(Object obj, AppSecRequestContext requestContext) {
71+
State state = new State(requestContext);
72+
Object converted = guardedConversion(obj, 0, state);
73+
if (state.stringTooLong || state.listMapTooLarge || state.objectTooDeep) {
74+
requestContext.setWafTruncated();
75+
WafMetricCollector.get()
76+
.wafInputTruncated(state.stringTooLong, state.listMapTooLarge, state.objectTooDeep);
77+
}
78+
return converted;
7279
}
7380

7481
private static class State {
7582
int elemsLeft = MAX_ELEMENTS;
7683
int invalidKeyId;
84+
boolean objectTooDeep = false;
85+
boolean listMapTooLarge = false;
86+
boolean stringTooLong = false;
87+
AppSecRequestContext requestContext;
88+
89+
private State(AppSecRequestContext requestContext) {
90+
this.requestContext = requestContext;
91+
}
7792
}
7893

7994
private static Object guardedConversion(Object obj, int depth, State state) {
@@ -94,31 +109,48 @@ private static String keyConversion(Object key, State state) {
94109
return "null";
95110
}
96111
if (key instanceof String) {
97-
return (String) key;
112+
return checkStringLength((String) key, state);
98113
}
99114
if (key instanceof Number
100115
|| key instanceof Boolean
101116
|| key instanceof Character
102117
|| key instanceof CharSequence) {
103-
return key.toString();
118+
return checkStringLength(key.toString(), state);
104119
}
105120
return "invalid_key:" + (++state.invalidKeyId);
106121
}
107122

108123
private static Object doConversion(Object obj, int depth, State state) {
124+
if (obj == null) {
125+
return null;
126+
}
109127
state.elemsLeft--;
110-
if (state.elemsLeft <= 0 || obj == null || depth > MAX_DEPTH) {
128+
if (state.elemsLeft <= 0) {
129+
state.listMapTooLarge = true;
130+
return null;
131+
}
132+
133+
if (depth > MAX_DEPTH) {
134+
state.objectTooDeep = true;
111135
return null;
112136
}
113137

114-
// strings, booleans and numbers are preserved
115-
if (obj instanceof String || obj instanceof Boolean || obj instanceof Number) {
138+
// booleans and numbers are preserved
139+
if (obj instanceof Boolean || obj instanceof Number) {
116140
return obj;
117141
}
118142

143+
// strings are preserved, but we need to check the length
144+
if (obj instanceof String) {
145+
return checkStringLength((String) obj, state);
146+
}
147+
119148
// char sequences are transformed just in case they are not immutable,
149+
if (obj instanceof CharSequence) {
150+
return checkStringLength(obj.toString(), state);
151+
}
120152
// single char sequences are transformed to strings for ddwaf compatibility.
121-
if (obj instanceof CharSequence || obj instanceof Character) {
153+
if (obj instanceof Character) {
122154
return obj.toString();
123155
}
124156

@@ -147,6 +179,7 @@ private static Object doConversion(Object obj, int depth, State state) {
147179
}
148180
for (Object o : ((Iterable<?>) obj)) {
149181
if (state.elemsLeft <= 0) {
182+
state.listMapTooLarge = true;
150183
break;
151184
}
152185
newList.add(guardedConversion(o, depth + 1, state));
@@ -178,6 +211,7 @@ private static Object doConversion(Object obj, int depth, State state) {
178211
for (Field[] fields : allFields) {
179212
for (Field f : fields) {
180213
if (state.elemsLeft <= 0) {
214+
state.listMapTooLarge = true;
181215
break outer;
182216
}
183217
if (Modifier.isStatic(f.getModifiers())) {
@@ -239,4 +273,12 @@ private static boolean setAccessible(Field field) {
239273
return false;
240274
}
241275
}
276+
277+
private static String checkStringLength(final String str, final State state) {
278+
if (str.length() > MAX_STRING_LENGTH) {
279+
state.stringTooLong = true;
280+
return str.substring(0, MAX_STRING_LENGTH);
281+
}
282+
return str;
283+
}
242284
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ private Flow<Void> onGrpcServerRequestMessage(RequestContext ctx_, Object obj) {
472472
if (subInfo == null || subInfo.isEmpty()) {
473473
return NoopFlow.INSTANCE;
474474
}
475-
Object convObj = ObjectIntrospection.convert(obj);
475+
Object convObj = ObjectIntrospection.convert(obj, ctx);
476476
DataBundle bundle =
477477
new SingletonDataBundle<>(KnownAddresses.GRPC_SERVER_REQUEST_MESSAGE, convObj);
478478
try {
@@ -574,7 +574,7 @@ private Flow<Void> onRequestBodyProcessed(RequestContext ctx_, Object obj) {
574574
}
575575
DataBundle bundle =
576576
new SingletonDataBundle<>(
577-
KnownAddresses.REQUEST_BODY_OBJECT, ObjectIntrospection.convert(obj));
577+
KnownAddresses.REQUEST_BODY_OBJECT, ObjectIntrospection.convert(obj, ctx));
578578
try {
579579
GatewayContext gwCtx = new GatewayContext(false);
580580
return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx);

0 commit comments

Comments
 (0)