Skip to content

Commit 03ad484

Browse files
MariovidoMariovido
authored
Expand SSRF support in IAST to apache-httpclient, commons-httpclient and okhttp (#7792)
1 parent 104a441 commit 03ad484

File tree

29 files changed

+497
-592
lines changed

29 files changed

+497
-592
lines changed

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
import datadog.trace.api.Config;
88
import datadog.trace.api.DDTags;
9+
import datadog.trace.api.InstrumenterConfig;
10+
import datadog.trace.api.ProductActivation;
11+
import datadog.trace.api.iast.InstrumentationBridge;
12+
import datadog.trace.api.iast.sink.SsrfModule;
913
import datadog.trace.api.naming.SpanNaming;
1014
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1115
import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes;
@@ -89,6 +93,8 @@ public AgentSpan onRequest(final AgentSpan span, final REQUEST request) {
8993
log.debug("Error tagging url", e);
9094
}
9195

96+
ssrfIastCheck(request);
97+
9298
if (CLIENT_TAG_HEADERS) {
9399
for (Map.Entry<String, String> headerTag :
94100
traceConfig(span).getRequestHeaderTags().entrySet()) {
@@ -168,4 +174,23 @@ public long getResponseContentLength(final RESPONSE response) {
168174

169175
return 0;
170176
}
177+
178+
/* This method must be overriden after making the proper propagations to the client before **/
179+
protected Object sourceUrl(REQUEST request) {
180+
return null;
181+
}
182+
183+
private void ssrfIastCheck(final REQUEST request) {
184+
final Object sourceUrl = sourceUrl(request);
185+
if (sourceUrl == null) {
186+
return;
187+
}
188+
if (InstrumenterConfig.get().getIastActivation() != ProductActivation.FULLY_ENABLED) {
189+
return;
190+
}
191+
final SsrfModule ssrfModule = InstrumentationBridge.SSRF;
192+
if (ssrfModule != null) {
193+
ssrfModule.onURLConnection(sourceUrl);
194+
}
195+
}
171196
}

‎dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package datadog.trace.bootstrap.instrumentation.decorator
22

33
import datadog.trace.api.DDTags
4+
import datadog.trace.api.iast.InstrumentationBridge
5+
import datadog.trace.api.iast.sink.SsrfModule
46
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
57
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
68
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities
@@ -171,6 +173,32 @@ class HttpClientDecoratorTest extends ClientDecoratorTest {
171173
null | null | false
172174
}
173175

176+
def "test ssrfIastCheck is called"() {
177+
setup:
178+
injectSysConfig('dd.iast.enabled', input)
179+
def decorator = newDecorator()
180+
final module = Mock(SsrfModule)
181+
InstrumentationBridge.registerIastModule(module)
182+
183+
when:
184+
decorator.onRequest(span, req)
185+
186+
then:
187+
if (input == 'true') {
188+
1 * module.onURLConnection(_)
189+
} else {
190+
0 * module.onURLConnection(_)
191+
}
192+
if (req) {
193+
1 * span.traceConfig() >> AgentTracer.traceConfig()
194+
}
195+
196+
where:
197+
input | req
198+
'true' | [method: "test-method", url: testUrl, path: '/somepath']
199+
'false' | [method: "test-method", url: testUrl, path: '/somepath']
200+
}
201+
174202
@Override
175203
def newDecorator(String serviceName = "test-service") {
176204
return new HttpClientDecorator<Map, Map>() {
@@ -199,6 +227,11 @@ class HttpClientDecoratorTest extends ClientDecoratorTest {
199227
return m.url
200228
}
201229

230+
@Override
231+
protected String sourceUrl(Map m) {
232+
return m.url
233+
}
234+
202235
@Override
203236
protected int status(Map m) {
204237
null == m.status ? 0 : m.status.intValue()

‎dd-java-agent/agent-iast/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ ext {
8686
excludedClassesCoverage = [
8787
// Avoid coverage measurement of model getters atm
8888
'com.datadog.iast.model.Evidence',
89+
'com.datadog.iast.sink.SinkModuleBase.EvidenceBuilder',
8990
'com.datadog.iast.model.Range',
9091
'com.datadog.iast.model.Source',
9192
'com.datadog.iast.model.Vulnerability',

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

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
package com.datadog.iast.sink;
22

33
import com.datadog.iast.Dependencies;
4-
import com.datadog.iast.model.Range;
54
import com.datadog.iast.model.VulnerabilityType;
6-
import com.datadog.iast.taint.Ranges;
7-
import com.datadog.iast.util.Iterators;
8-
import com.datadog.iast.util.RangeBuilder;
95
import datadog.trace.api.iast.sink.SsrfModule;
106
import javax.annotation.Nullable;
117

@@ -22,64 +18,4 @@ public void onURLConnection(@Nullable final Object url) {
2218
}
2319
checkInjection(VulnerabilityType.SSRF, url);
2420
}
25-
26-
/**
27-
* if the host or the uri are tainted, we report the url as tainted as well a new range is created
28-
* covering all the value string in order to simplify the algorithm
29-
*/
30-
@Override
31-
public void onURLConnection(@Nullable String value, @Nullable Object host, @Nullable Object uri) {
32-
if (value == null) {
33-
return;
34-
}
35-
checkInjection(VulnerabilityType.SSRF, Iterators.of(host, uri), new SsrfEvidenceBuilder(value));
36-
}
37-
38-
private static class SsrfEvidenceBuilder implements EvidenceBuilder {
39-
40-
private final String url;
41-
42-
private SsrfEvidenceBuilder(final String url) {
43-
this.url = url;
44-
}
45-
46-
@Override
47-
public void tainted(
48-
final StringBuilder evidence,
49-
final RangeBuilder ranges,
50-
final Object value,
51-
final Range[] valueRanges) {
52-
if (!ranges.isEmpty()) {
53-
return; // skip if already tainted
54-
}
55-
evidence.append(url);
56-
if (value != null && url.equals(value.toString())) {
57-
// safe path, the URL is exactly the same
58-
ranges.add(valueRanges);
59-
} else {
60-
final Range range = Ranges.highestPriorityRange(valueRanges);
61-
final String tainted = substring(value, range);
62-
final int offset = tainted == null ? -1 : url.indexOf(tainted);
63-
if (offset >= 0) {
64-
// use the specific part of the tainted URL
65-
ranges.add(range.shift(offset));
66-
} else {
67-
// resort to fully taint the URL
68-
ranges.add(Ranges.forCharSequence(url, range.getSource()));
69-
}
70-
}
71-
}
72-
73-
@Nullable
74-
private String substring(Object value, final Range range) {
75-
if (value == null) {
76-
return null;
77-
}
78-
final String stringValue = value.toString();
79-
if (range.getStart() + range.getLength() > stringValue.length()) {
80-
return null;
81-
}
82-
return stringValue.substring(range.getStart(), range.getStart() + range.getLength());
83-
}
84-
}
8521
}

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

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ import datadog.trace.api.iast.SourceTypes
1111
import datadog.trace.api.iast.VulnerabilityMarks
1212
import datadog.trace.api.iast.sink.SsrfModule
1313

14-
import static com.datadog.iast.taint.TaintUtils.fromTaintFormat
15-
import static com.datadog.iast.taint.TaintUtils.getStringFromTaintFormat
16-
import static com.datadog.iast.taint.TaintUtils.taintFormat
17-
1814
class SsrfModuleTest extends IastModuleImplTestBase {
1915

2016
private SsrfModule module
@@ -36,14 +32,6 @@ class SsrfModuleTest extends IastModuleImplTestBase {
3632
0 * _
3733
}
3834

39-
void 'test null value'() {
40-
when:
41-
module.onURLConnection(null, null, null)
42-
43-
then:
44-
0 * _
45-
}
46-
4735
void 'test SSRF detection'() {
4836
when:
4937
module.onURLConnection(url)
@@ -89,68 +77,7 @@ class SsrfModuleTest extends IastModuleImplTestBase {
8977
0 * reporter.report(_, _)
9078
}
9179

92-
void 'test SSRF detection for host and uri'() {
93-
when:
94-
module.onURLConnection(value, host, uri)
95-
96-
then: 'report is not called if no active span'
97-
tracer.activeSpan() >> null
98-
0 * reporter.report(_, _)
99-
100-
when:
101-
module.onURLConnection(value, host, uri)
102-
103-
then: 'report is not called if host or uri are not tainted'
104-
tracer.activeSpan() >> span
105-
0 * reporter.report(_, _)
106-
107-
when:
108-
host = tainted(host)
109-
uri = taintedURI(uri)
110-
module.onURLConnection(value, host, uri)
111-
112-
then: 'report is called when the host or uri are tainted'
113-
tracer.activeSpan() >> span
114-
if (expected == null) {
115-
0 * reporter.report(_, _)
116-
} else {
117-
1 * reporter.report(span, { Vulnerability vul ->
118-
vul.type == VulnerabilityType.SSRF && taintFormat(vul.evidence.value, vul.evidence.ranges) == expected
119-
})
120-
}
121-
122-
where:
123-
value | host | uri | expected
124-
'http://test.com/tested?1=1' | 'test.com' | 'http://test.com/tested?1=1' | null
125-
'http://test.com/tested?1=1' | '==>test.com<==' | 'http://test.com/tested?1=1' | 'http://==>test.com<==/tested?1=1'
126-
'http://test.com/tested?1=1' | '==>another.com<==' | 'http://test.com/tested?1=1' | '==>http://test.com/tested?1=1<==' // no match so full taint
127-
'http://test.com/tested?1=1' | 'test.com' | '==>http://test.com/tested?1=1<==' | '==>http://test.com/tested?1=1<=='
128-
'http://test.com/tested?1=1' | 'test.com' | '==>http<==://test.com/tested?1=1' | '==>http<==://test.com/tested?1=1'
129-
'http://test.com/tested?1=1' | 'test.com' | 'http://==>test.com<==/tested?1=1' | 'http://==>test.com<==/tested?1=1'
130-
'http://test.com/tested?1=1' | 'test.com' | 'http://test.com/==>tested<==?1=1' | 'http://test.com/==>tested<==?1=1'
131-
'http://test.com/tested?1=1' | 'test.com' | 'http://test.com/tested==>?1=1<==' | 'http://test.com/tested==>?1=1<=='
132-
}
133-
13480
private taint(final Object value) {
13581
ctx.getTaintedObjects().taint(value, Ranges.forObject(new Source(SourceTypes.REQUEST_PARAMETER_VALUE, 'name', value.toString())))
13682
}
137-
138-
139-
private String tainted(final String url) {
140-
final uri = getStringFromTaintFormat(url)
141-
final ranges = fromTaintFormat(url)
142-
if (ranges?.length > 0) {
143-
ctx.getTaintedObjects().taint(uri, ranges)
144-
}
145-
return uri
146-
}
147-
148-
private URI taintedURI(final String url) {
149-
final uri = new URI(getStringFromTaintFormat(url))
150-
final ranges = fromTaintFormat(url)
151-
if (ranges?.length > 0) {
152-
ctx.getTaintedObjects().taint(uri, ranges)
153-
}
154-
return uri
155-
}
15683
}

‎dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ protected URI url(final HttpUriRequest request) {
3434
return request.getURI();
3535
}
3636

37+
@Override
38+
protected URI sourceUrl(final HttpUriRequest request) {
39+
return request.getURI();
40+
}
41+
3742
@Override
3843
protected int status(final HttpResponse httpResponse) {
3944
return httpResponse.getStatusLine().getStatusCode();

0 commit comments

Comments
 (0)