Skip to content

Commit 0b9fa7f

Browse files
committed
Fix ObjectIntrospection exposing JDK internal toString() to the WAF
1 parent ea11e60 commit 0b9fa7f

2 files changed

Lines changed: 55 additions & 9 deletions

File tree

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,13 @@ private static Object doConversion(Object obj, int depth, State state) {
303303
// TODO: Use invalid object
304304
}
305305
} else {
306-
// One of fields is inaccessible, might be it's Strongly Encapsulated Internal class
307-
// consider it as integral object without introspection
308-
// TODO: Use invalid object
309-
return obj.toString();
306+
// This field is inaccessible (Strongly Encapsulated Internal class on Java 9+).
307+
// Skip it and continue with the remaining fields — other accessible fields on the
308+
// same object may still contain useful data for WAF inspection. Do NOT call
309+
// obj.toString() here: JDK internal toString() representations (e.g.
310+
// "class java.lang.Object") can match legitimate WAF phrase_match rules and
311+
// produce false positives (e.g. crs-944-130 java_code_injection).
312+
continue;
310313
}
311314
}
312315
}

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

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
164164
void 'max number of elements is honored'() {
165165
setup:
166166
def m = [:]
167-
128.times { m[it] = 'b' }
167+
128.times {
168+
m[it] = 'b'
169+
}
168170

169171
when:
170172
def result1 = convert([['a'] * 255], ctx)[0]
@@ -184,7 +186,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
184186
// Build a nested array 22 levels deep
185187
Object[] objArray = new Object[1]
186188
def p = objArray
187-
22.times { p = p[0] = new Object[1] }
189+
22.times {
190+
p = p[0] = new Object[1]
191+
}
188192

189193
when:
190194
// Invoke conversion with context
@@ -208,7 +212,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
208212
// Build a nested list 22 levels deep
209213
def list = []
210214
def p = list
211-
22.times { p << []; p = p[0] }
215+
22.times {
216+
p << []; p = p[0]
217+
}
212218

213219
when:
214220
// Invoke conversion with context
@@ -232,7 +238,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
232238
// Build a nested map 22 levels deep under key 'a'
233239
def map = [:]
234240
def p = map
235-
22.times { p['a'] = [:]; p = p['a'] }
241+
22.times {
242+
p['a'] = [:]; p = p['a']
243+
}
236244

237245
when:
238246
// Invoke conversion with context
@@ -413,7 +421,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
413421
setup:
414422
// Create deeply nested JSON
415423
final json = JsonOutput.toJson(
416-
(1..(MAX_DEPTH + 1)).inject([:], { result, i -> [("child_$i".toString()) : result] })
424+
(1..(MAX_DEPTH + 1)).inject([:], {
425+
result, i -> [("child_$i".toString()) : result]
426+
})
417427
)
418428

419429
when:
@@ -478,6 +488,39 @@ class ObjectIntrospectionSpecification extends DDSpecification {
478488
MAPPER.readTree('"unicode: \\u0041"') || 'unicode: A'
479489
}
480490

491+
void 'objects with inaccessible JDK fields skip those fields rather than expose toString()'() {
492+
given: '''An object with two fields: one normal, one holding a java.lang.ref.SoftReference.
493+
java.lang.ref is NOT opened in the test JVM (only java.lang and java.util are),
494+
so trySetAccessible() returns false for SoftReference's own fields on Java 9+.
495+
496+
Bug: ObjectIntrospection fell back to obj.toString() when any field was inaccessible,
497+
exposing internal JDK string representations to the WAF and discarding all other
498+
accessible fields on the same object. For example, java.lang.Class.toString() produces
499+
"class java.lang.Object" which matches WAF phrase_match rule crs-944-130
500+
(java_code_injection) — a false positive causing a CPU spike.
501+
502+
Fix: skip inaccessible fields (continue) instead of aborting the whole object.
503+
Accessible fields on the same object are still reported to the WAF.'''
504+
def input = new WrapperWithSoftRef()
505+
506+
when:
507+
def result = convert(input, ctx) as Map
508+
509+
then:
510+
// The accessible field 'name' must be preserved
511+
result['name'] == 'test'
512+
// The inaccessible-field object must NOT expose toString() — it should be an empty Map
513+
// (all fields of java.lang.ref.SoftReference/Reference are inaccessible without --add-opens)
514+
// Before fix: result['ref'] == "java.lang.ref.SoftReference@..." (false WAF positive)
515+
// After fix: result['ref'] == [:] (empty map — object present, no accessible fields)
516+
result['ref'] == [:]
517+
}
518+
519+
static class WrapperWithSoftRef {
520+
String name = 'test'
521+
java.lang.ref.SoftReference<String> ref = new java.lang.ref.SoftReference<>("test")
522+
}
523+
481524
void 'iterable json objects'() {
482525
setup:
483526
final map = [name: 'This is just a test', list: [1, 2, 3, 4, 5]]

0 commit comments

Comments
 (0)