Fix ObjectIntrospection exposing JDK internal toString() to the WAF (#10820) (#10882)

Backport #10820 to release/v1.60.x

Author: jandro996

dd-java-agent/appsec/build.gradle

@@ -9,6 +9,7 @@ plugins {
 
 apply from: "$rootDir/gradle/java.gradle"
 apply from: "$rootDir/gradle/version.gradle"
+apply from: "$rootDir/gradle/tries.gradle"
 
 dependencies {
   api libs.slf4j
@@ -29,6 +30,14 @@ dependencies {
   testImplementation libs.jackson.databind
 }
 
+tasks.named("compileJava", JavaCompile) {
+  dependsOn("generateClassNameTries")
+}
+
+tasks.named("sourcesJar", Jar) {
+  dependsOn("generateClassNameTries")
+}
+
 tasks.named("shadowJar", ShadowJar) {
   exclude '**/*-dbgsym.zip'
   dependencies deps.excludeShared

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

@@ -287,7 +287,7 @@ private static Object doConversion(Object obj, int depth, State state) {
         if (Modifier.isStatic(f.getModifiers())) {
           continue;
         }
-        if (f.getType().getName().equals("groovy.lang.MetaClass")) {
+        if (IntrospectionExcludedTypesTrie.apply(f.getType().getName()) >= 1) {
           continue;
         }
         String name = f.getName();
@@ -302,12 +302,13 @@ private static Object doConversion(Object obj, int depth, State state) {
             log.error("Unable to get field value", e);
             // TODO: Use invalid object
           }
-        } else {
-          // One of fields is inaccessible, might be it's Strongly Encapsulated Internal class
-          // consider it as integral object without introspection
-          // TODO: Use invalid object
-          return obj.toString();
         }
+        // This field is inaccessible (Strongly Encapsulated Internal class on Java 9+).
+        // Skip it and continue with the remaining fields — other accessible fields on the
+        // same object may still contain useful data for WAF inspection. Do NOT call
+        // obj.toString() here: JDK internal toString() representations (e.g.
+        // "class java.lang.Object") can match legitimate WAF phrase_match rules and
+        // produce false positives (e.g. crs-944-130 java_code_injection).
       }
     }
 

dd-java-agent/appsec/src/main/resources/com/datadog/appsec/event/data/introspection_excluded_types.trie

@@ -0,0 +1,15 @@
+# Generates 'IntrospectionExcludedTypesTrie.java'
+
+# Field types excluded from ObjectIntrospection to avoid deep/cyclic traversals
+# that could trigger WAF false positives (e.g. crs-944-130 java_code_injection).
+# 1 = exclude this field type
+
+# -------- Groovy --------
+1 groovy.lang.MetaClass
+
+# -------- Logging frameworks --------
+1 ch.qos.logback.*
+1 java.util.logging.Logger
+1 org.apache.commons.logging.Log
+1 org.apache.logging.*
+1 org.slf4j.*

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

@@ -164,7 +164,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
   void 'max number of elements is honored'() {
     setup:
     def m = [:]
-    128.times { m[it] = 'b' }
+    128.times {
+      m[it] = 'b'
+    }
 
     when:
     def result1 = convert([['a'] * 255], ctx)[0]
@@ -184,7 +186,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
     // Build a nested array 22 levels deep
     Object[] objArray = new Object[1]
     def p = objArray
-    22.times { p = p[0] = new Object[1] }
+    22.times {
+      p = p[0] = new Object[1]
+    }
 
     when:
     // Invoke conversion with context
@@ -208,7 +212,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
     // Build a nested list 22 levels deep
     def list = []
     def p = list
-    22.times { p << []; p = p[0] }
+    22.times {
+      p << []; p = p[0]
+    }
 
     when:
     // Invoke conversion with context
@@ -232,7 +238,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
     // Build a nested map 22 levels deep under key 'a'
     def map = [:]
     def p = map
-    22.times { p['a'] = [:]; p = p['a'] }
+    22.times {
+      p['a'] = [:]; p = p['a']
+    }
 
     when:
     // Invoke conversion with context
@@ -413,7 +421,9 @@ class ObjectIntrospectionSpecification extends DDSpecification {
     setup:
     // Create deeply nested JSON
     final json = JsonOutput.toJson(
-    (1..(MAX_DEPTH + 1)).inject([:], { result, i -> [("child_$i".toString()) : result] })
+    (1..(MAX_DEPTH + 1)).inject([:], {
+      result, i -> [("child_$i".toString()) : result]
+    })
     )
 
     when:
@@ -478,6 +488,74 @@ class ObjectIntrospectionSpecification extends DDSpecification {
     MAPPER.readTree('"unicode: \\u0041"')          || 'unicode: A'
   }
 
+  void 'logging framework fields are excluded from introspection'() {
+    given: '''Reproduces the false positive triggered by crs-944-130 (java_code_injection).
+             A request body DTO had an instance field of a concrete logging type. ObjectIntrospection
+             traversed the logger internals 19 levels deep (SLF4J → Log4jLogger → core.Logger →
+             privateConfig → loggerConfig → appenders → appenderArray → appender → manager →
+             layout → objectWriter → _config → _base → _typeFactory → _typeCache → _map →
+             invalid_key:9 → _elementType → _class), eventually calling
+             java.lang.Class.toString() = "class java.lang.object" which phrase-matched the WAF rule.
+             The field in the real trace was declared as org.slf4j.Logger (runtime: Log4jLogger),
+             but the trie also covers org.apache.logging.slf4j.Log4jLogger directly via
+             org.apache.logging.* in case the field is declared as the concrete class.'''
+    def input = new DtoWithLogger()
+
+    when:
+    def result = convert(input, ctx) as Map
+
+    then:
+    result['userId'] == 'user123'
+    result['payload'] == 'data'
+    !result.containsKey('logger')
+  }
+
+  static class DtoWithLogger {
+    String userId = 'user123'
+    // Declared as the concrete Logback type (ch.qos.logback.classic.Logger) to mirror the
+    // real scenario where org.apache.logging.slf4j.Log4jLogger is the runtime instance of
+    // a field declared as the concrete logger class rather than the SLF4J interface.
+    // Both are covered by the trie (ch.qos.logback.* and org.apache.logging.*).
+    ch.qos.logback.classic.Logger logger = (ch.qos.logback.classic.Logger) org.slf4j.LoggerFactory.getLogger('test')
+    String payload = 'data'
+  }
+
+  void 'objects with inaccessible JDK fields skip those fields rather than expose toString()'() {
+    given: '''An object with two fields: one normal, one holding a java.lang.ref.SoftReference.
+             java.lang.ref is NOT opened in the test JVM (only java.lang and java.util are),
+             so trySetAccessible() returns false for SoftReference's own fields on Java 9+.
+
+             Bug: ObjectIntrospection fell back to obj.toString() when any field was inaccessible,
+             exposing internal JDK string representations to the WAF and discarding all other
+             accessible fields on the same object. For example, java.lang.Class.toString() produces
+             "class java.lang.Object" which matches WAF phrase_match rule crs-944-130
+             (java_code_injection) — a false positive causing a CPU spike.
+
+             Fix: skip inaccessible fields (continue) instead of aborting the whole object.
+             Accessible fields on the same object are still reported to the WAF.'''
+    def input = new WrapperWithSoftRef()
+
+    when:
+    def result = convert(input, ctx) as Map
+
+    then:
+    // The accessible field 'name' must be preserved regardless of JVM version
+    result['name'] == 'test'
+    // The inaccessible-field object must NOT expose toString() to the WAF.
+    // On JDK 8 and JDK 9-15 (--illegal-access=permit default), java.lang.ref fields are
+    // accessible so result['ref'] is a non-empty Map. On JDK 16+ strict module enforcement,
+    // result['ref'] is an empty Map [:] since all Reference fields are inaccessible.
+    // Before fix (any version where fields are inaccessible): result['ref'] is a String
+    //   e.g. "java.lang.ref.SoftReference@..." — a false WAF positive.
+    // After fix: result['ref'] is always a Map, never a String.
+    result['ref'] instanceof Map
+  }
+
+  static class WrapperWithSoftRef {
+    String name = 'test'
+    java.lang.ref.SoftReference<String> ref = new java.lang.ref.SoftReference<>("test")
+  }
+
   void 'iterable json objects'() {
     setup:
     final map = [name: 'This is just a test', list: [1, 2, 3, 4, 5]]