From 0305b4078a510b36168f978baebe83021f97bdbb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 12 Mar 2026 15:25:30 +0100 Subject: [PATCH 1/4] Fix ObjectIntrospection exposing JDK internal toString() to the WAF --- .../event/data/ObjectIntrospection.java | 11 ++-- .../ObjectIntrospectionSpecification.groovy | 53 +++++++++++++++++-- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java index 9169e2f2e08..dbeb0834f06 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java @@ -303,10 +303,13 @@ private static Object doConversion(Object obj, int depth, State state) { // 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). + continue; } } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index f945244cf03..280d52f9ac4 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/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,39 @@ class ObjectIntrospectionSpecification extends DDSpecification { MAPPER.readTree('"unicode: \\u0041"') || 'unicode: A' } + 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 + result['name'] == 'test' + // The inaccessible-field object must NOT expose toString() — it should be an empty Map + // (all fields of java.lang.ref.SoftReference/Reference are inaccessible without --add-opens) + // Before fix: result['ref'] == "java.lang.ref.SoftReference@..." (false WAF positive) + // After fix: result['ref'] == [:] (empty map — object present, no accessible fields) + result['ref'] == [:] + } + + static class WrapperWithSoftRef { + String name = 'test' + java.lang.ref.SoftReference 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]] From b2b247baa304552758788345b4715e9a8c23af6c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 12 Mar 2026 15:42:33 +0100 Subject: [PATCH 2/4] wip --- .../appsec/event/data/ObjectIntrospection.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java index dbeb0834f06..3eb05726a85 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java @@ -302,15 +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 { - // 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). - continue; } + // 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). } } From c81802884ff3a78292107b37693b3bc34132059a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 12 Mar 2026 16:12:04 +0100 Subject: [PATCH 3/4] fix test for all jdks --- .../data/ObjectIntrospectionSpecification.groovy | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index 280d52f9ac4..23c7dc53460 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy @@ -507,13 +507,16 @@ class ObjectIntrospectionSpecification extends DDSpecification { def result = convert(input, ctx) as Map then: - // The accessible field 'name' must be preserved + // The accessible field 'name' must be preserved regardless of JVM version result['name'] == 'test' - // The inaccessible-field object must NOT expose toString() — it should be an empty Map - // (all fields of java.lang.ref.SoftReference/Reference are inaccessible without --add-opens) - // Before fix: result['ref'] == "java.lang.ref.SoftReference@..." (false WAF positive) - // After fix: result['ref'] == [:] (empty map — object present, no accessible fields) - result['ref'] == [:] + // 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 { From 208c7bf1a717ce77770b256088adac996c5dafd4 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Mar 2026 15:26:07 +0100 Subject: [PATCH 4/4] Avoid log classes --- .../event/data/ObjectIntrospection.java | 17 +++++++++++++ .../ObjectIntrospectionSpecification.groovy | 24 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java index 3eb05726a85..4685e629025 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/ObjectIntrospection.java @@ -290,6 +290,9 @@ private static Object doConversion(Object obj, int depth, State state) { if (f.getType().getName().equals("groovy.lang.MetaClass")) { continue; } + if (isLoggingType(f.getType())) { + continue; + } String name = f.getName(); if (ignoredFieldName(name)) { continue; @@ -315,6 +318,20 @@ private static Object doConversion(Object obj, int depth, State state) { return newMap; } + private static boolean isLoggingType(final Class type) { + switch (type.getName()) { + case "org.slf4j.Logger": + case "org.apache.logging.log4j.Logger": + case "org.apache.logging.log4j.core.Logger": + case "java.util.logging.Logger": + case "org.apache.commons.logging.Log": + case "ch.qos.logback.classic.Logger": + return true; + default: + return false; + } + } + private static boolean ignoredFieldName(final String name) { switch (name) { case "this$0": diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy index 23c7dc53460..c02459fb0d6 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/ObjectIntrospectionSpecification.groovy @@ -488,6 +488,30 @@ class ObjectIntrospectionSpecification extends DDSpecification { MAPPER.readTree('"unicode: \\u0041"') || 'unicode: A' } + void 'logging framework fields are excluded from introspection'() { + given: 'a DTO with a logger instance field alongside regular fields' + // Reproduces the false positive in crs-944-130: an instance logger field on the request + // body DTO causes ObjectIntrospection to traverse logging framework internals (Log4j, + // Jackson TypeCache), eventually hitting java.lang.Class.toString() = "class java.lang.Object" + // which matches the WAF phrase_match rule. Skipping logging-typed fields cuts the traversal + // at the root and eliminates the false positive without discarding other useful fields. + 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' + java.util.logging.Logger logger = java.util.logging.Logger.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),