Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions dd-java-agent/appsec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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).
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.*
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]]
Expand Down
Loading