Skip to content

Commit 3fd69de

Browse files
Ensure vulnerabilities are reported with taintable values (#7801)
1 parent a11c206 commit 3fd69de

File tree

3 files changed

+95
-5
lines changed

3 files changed

+95
-5
lines changed

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/Source.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,18 @@ public String getValue() {
5252
return asString(value);
5353
}
5454

55+
/** This will expose the internal reference so be careful */
56+
@Nullable
57+
public Object getRawValue() {
58+
if (value == null) {
59+
return null;
60+
}
61+
if (value instanceof Reference<?>) {
62+
return ((Reference<?>) value).get();
63+
}
64+
return value;
65+
}
66+
5567
@SuppressFBWarnings(
5668
value = "DM_STRING_CTOR",
5769
justification = "New string instance requires constructor")

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import datadog.trace.api.Config;
2222
import datadog.trace.api.Pair;
2323
import datadog.trace.api.iast.IastContext;
24+
import datadog.trace.api.iast.Taintable;
2425
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
2526
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
2627
import datadog.trace.instrumentation.iastinstrumenter.IastExclusionTrie;
@@ -108,17 +109,35 @@ protected final Evidence checkInjection(
108109
protected final Evidence checkInjection(
109110
final IastContext ctx,
110111
final VulnerabilityType type,
111-
final Object value,
112+
Object value,
112113
@Nullable final EvidenceBuilder evidenceBuilder,
113114
@Nullable final LocationSupplier locationSupplier) {
114115

115116
final TaintedObjects to = ctx.getTaintedObjects();
116-
final TaintedObject tainted = to.get(value);
117-
if (tainted == null) {
118-
return null;
117+
final Range[] valueRanges;
118+
if (value instanceof Taintable) {
119+
final Taintable taintable = (Taintable) value;
120+
if (!taintable.$DD$isTainted()) {
121+
return null;
122+
}
123+
final Source source = (Source) taintable.$$DD$getSource();
124+
final Object origin = source.getRawValue();
125+
final TaintedObject tainted = origin == null ? null : to.get(origin);
126+
if (origin != null && tainted != null) {
127+
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
128+
value = origin;
129+
} else {
130+
valueRanges = Ranges.forObject((Source) taintable.$$DD$getSource(), type.mark());
131+
value = String.format("Tainted reference detected in " + value.getClass());
132+
}
133+
} else {
134+
final TaintedObject tainted = to.get(value);
135+
if (tainted == null) {
136+
return null;
137+
}
138+
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
119139
}
120140

121-
final Range[] valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
122141
if (valueRanges == null || valueRanges.length == 0) {
123142
return null;
124143
}

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/AbstractSinkModuleTest.groovy

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import com.datadog.iast.model.Source
55
import com.datadog.iast.overhead.Operations
66
import com.datadog.iast.propagation.PropagationModuleImpl
77
import com.datadog.iast.taint.Ranges
8+
import datadog.trace.api.iast.Taintable
9+
10+
import java.lang.ref.WeakReference
811

912
import static com.datadog.iast.model.VulnerabilityType.SSRF
1013
import static datadog.trace.api.iast.SourceTypes.REQUEST_PARAMETER_VALUE
@@ -80,7 +83,63 @@ class AbstractSinkModuleTest extends IastModuleImplTestBase {
8083
new Source(REQUEST_PARAMETER_VALUE, 'url', 'datadog.com') | new URI('https://dAtAdOg.com/index.html') | false
8184
}
8285

86+
void 'test reporting with taintables'() {
87+
setup:
88+
final sink = new SinkModuleBase(dependencies) {}
89+
final value = 'datadog.com'
90+
final valueRef = new WeakReference<>(value)
91+
final source = new Source(REQUEST_PARAMETER_VALUE, 'url', valueRef)
92+
93+
and:
94+
final taintable = new MockTaintable(source: source)
95+
96+
when: 'original source value is not tainted'
97+
def evidence = sink.checkInjection(SSRF, taintable)
98+
99+
then: 'a fallback evidence is provided'
100+
evidence.value == "Tainted reference detected in " + taintable.class
101+
evidence.ranges.length == 1
102+
evidence.ranges[0].start == 0
103+
evidence.ranges[0].length == evidence.value.length()
104+
105+
when: 'original source value is tainted'
106+
ctx.getTaintedObjects().taint(value, Ranges.forCharSequence(value, source))
107+
evidence = sink.checkInjection(SSRF, taintable)
108+
109+
then: 'the proper value is set in the evidence'
110+
evidence.value == value
111+
evidence.ranges.length == 1
112+
evidence.ranges[0].start == 0
113+
evidence.ranges[0].length == value.length()
114+
115+
when: 'original source cleared by the GC'
116+
valueRef.clear()
117+
evidence = sink.checkInjection(SSRF, taintable)
118+
119+
then: 'a fallback evidence is provided'
120+
evidence.value == "Tainted reference detected in " + taintable.class
121+
evidence.ranges.length == 1
122+
evidence.ranges[0].start == 0
123+
evidence.ranges[0].length == evidence.value.length()
124+
}
125+
83126
private StackTraceElement element(final String declaringClass) {
84127
return new StackTraceElement(declaringClass, "method", "fileName", 1)
85128
}
129+
130+
private static class MockTaintable implements Taintable {
131+
private Source source
132+
133+
@SuppressWarnings('CodeNarc')
134+
@Override
135+
Source $$DD$getSource() {
136+
return source
137+
}
138+
139+
@SuppressWarnings('CodeNarc')
140+
@Override
141+
void $$DD$setSource(Source source) {
142+
this.source = source
143+
}
144+
}
86145
}

0 commit comments

Comments
 (0)