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

Fix ObjectIntrospection exposing JDK internal toString() to the WAF

wip

fix test for all jdks

Avoid log classes

new approach test

new approach test

change to .trie

WIP

Merge branch 'master' into alejandro.gonzalez/APPSEC-61693

Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

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