Skip to content

Commit ec18d0e

Browse files
DDJavierSantosDDJavierSantos
authored
Added NoHttpOnlyCookie Module and refactored InstrumentationBridge
1 parent ce8a316 commit ec18d0e

File tree

29 files changed

+368
-178
lines changed

29 files changed

+368
-178
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.datadog.iast.sink.CommandInjectionModuleImpl;
99
import com.datadog.iast.sink.InsecureCookieModuleImpl;
1010
import com.datadog.iast.sink.LdapInjectionModuleImpl;
11+
import com.datadog.iast.sink.NoHttpOnlyCookieModuleImpl;
1112
import com.datadog.iast.sink.PathTraversalModuleImpl;
1213
import com.datadog.iast.sink.SqlInjectionModuleImpl;
1314
import com.datadog.iast.sink.SsrfModuleImpl;
@@ -94,6 +95,7 @@ private static Stream<IastModule> iastModules() {
9495
new LdapInjectionModuleImpl(),
9596
new PropagationModuleImpl(),
9697
new InsecureCookieModuleImpl(),
98+
new NoHttpOnlyCookieModuleImpl(),
9799
new SsrfModuleImpl(),
98100
new UnvalidatedRedirectModuleImpl(),
99101
new WeakRandomnessModuleImpl());

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ public interface VulnerabilityType {
99
VulnerabilityType WEAK_CIPHER = new VulnerabilityTypeImpl(VulnerabilityTypes.WEAK_CIPHER);
1010
VulnerabilityType WEAK_HASH = new VulnerabilityTypeImpl(VulnerabilityTypes.WEAK_HASH);
1111
VulnerabilityType INSECURE_COOKIE = new VulnerabilityTypeImpl(VulnerabilityTypes.INSECURE_COOKIE);
12+
VulnerabilityType NO_HTTPONLY_COOKIE =
13+
new VulnerabilityTypeImpl(VulnerabilityTypes.NO_HTTPONLY_COOKIE);
1214
InjectionType SQL_INJECTION = new InjectionTypeImpl(VulnerabilityTypes.SQL_INJECTION, ' ');
1315
InjectionType COMMAND_INJECTION =
1416
new InjectionTypeImpl(VulnerabilityTypes.COMMAND_INJECTION, ' ');

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

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,61 +3,26 @@
33
import com.datadog.iast.model.Evidence;
44
import com.datadog.iast.model.VulnerabilityType;
55
import com.datadog.iast.overhead.Operations;
6-
import datadog.trace.api.Config;
76
import datadog.trace.api.iast.sink.InsecureCookieModule;
87
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
98
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
10-
import java.net.HttpCookie;
11-
import java.util.List;
129
import javax.annotation.Nonnull;
1310

1411
public class InsecureCookieModuleImpl extends SinkModuleBase implements InsecureCookieModule {
1512

16-
private Config config;
17-
18-
@Override
19-
public void registerDependencies(@Nonnull Dependencies dependencies) {
20-
super.registerDependencies(dependencies);
21-
config = dependencies.getConfig();
22-
}
23-
2413
@Override
25-
public void onCookie(String cookieName, boolean secure) {
26-
if (!secure && cookieName != null && cookieName.length() > 0) {
14+
public void onCookie(
15+
@Nonnull final String name,
16+
final String value,
17+
final boolean isSecure,
18+
final boolean isHttpOnly,
19+
final String sameSite) {
20+
if (!isSecure) {
2721
final AgentSpan span = AgentTracer.activeSpan();
2822
if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) {
2923
return;
3024
}
31-
report(span, VulnerabilityType.INSECURE_COOKIE, new Evidence(cookieName));
32-
}
33-
}
34-
35-
@Override
36-
public void onCookieHeader(String value) {
37-
if (null == value) {
38-
return;
39-
}
40-
try {
41-
List<HttpCookie> cookies = HttpCookie.parse(value);
42-
for (HttpCookie cookie : cookies) {
43-
if (!cookie.getSecure()) {
44-
final AgentSpan span = AgentTracer.activeSpan();
45-
if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) {
46-
return;
47-
}
48-
report(span, VulnerabilityType.INSECURE_COOKIE, new Evidence(cookie.getName()));
49-
return;
50-
}
51-
}
52-
} catch (IllegalArgumentException e) {
53-
return;
54-
}
55-
}
56-
57-
@Override
58-
public void onHeader(String name, String value) {
59-
if ("Set-Cookie".equalsIgnoreCase(name)) {
60-
onCookieHeader(value);
25+
report(span, VulnerabilityType.INSECURE_COOKIE, new Evidence(name));
6126
}
6227
}
6328
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.datadog.iast.sink;
2+
3+
import com.datadog.iast.model.Evidence;
4+
import com.datadog.iast.model.VulnerabilityType;
5+
import com.datadog.iast.overhead.Operations;
6+
import datadog.trace.api.iast.sink.NoHttpOnlyCookieModule;
7+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
8+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
9+
import javax.annotation.Nonnull;
10+
11+
public class NoHttpOnlyCookieModuleImpl extends SinkModuleBase implements NoHttpOnlyCookieModule {
12+
13+
@Override
14+
public void onCookie(
15+
@Nonnull final String name,
16+
final String value,
17+
final boolean isSecure,
18+
final boolean isHttpOnly,
19+
final String sameSite) {
20+
if (!isHttpOnly) {
21+
final AgentSpan span = AgentTracer.activeSpan();
22+
if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) {
23+
return;
24+
}
25+
report(span, VulnerabilityType.NO_HTTPONLY_COOKIE, new Evidence(name));
26+
}
27+
}
28+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
public class UnvalidatedRedirectModuleImpl extends SinkModuleBase
2020
implements UnvalidatedRedirectModule {
2121

22+
private static final String LOCATION_HEADER = "Location";
23+
2224
@Override
2325
public void onRedirect(final @Nullable String value) {
2426
if (!canBeTainted(value)) {
@@ -72,8 +74,8 @@ public void onURIRedirect(@Nullable URI uri) {
7274
}
7375

7476
@Override
75-
public void onHeader(String name, String value) {
76-
if ("Location".equalsIgnoreCase(name)) {
77+
public void onHeader(@Nonnull final String name, final String value) {
78+
if (value != null && LOCATION_HEADER.equalsIgnoreCase(name)) {
7779
onRedirect(value);
7880
}
7981
}

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

Lines changed: 19 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import com.datadog.iast.model.Vulnerability
66
import com.datadog.iast.model.VulnerabilityType
77
import datadog.trace.api.gateway.RequestContext
88
import datadog.trace.api.gateway.RequestContextSlot
9-
import datadog.trace.api.iast.InstrumentationBridge
109
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
1110
import groovy.transform.CompileDynamic
1211

@@ -34,12 +33,12 @@ class InsecureCookieModuleTest extends IastModuleImplTestBase {
3433
}
3534
}
3635

37-
void 'report insecure cookie with InsecureCookieModule.onCookieHeader'() {
36+
void 'report insecure cookie with InsecureCookieModule.onCookie'() {
3837
given:
3938
Vulnerability savedVul
4039

4140
when:
42-
module.onCookieHeader(cookieValue)
41+
module.onCookie('user-id', '7', false, false, null)
4342

4443
then:
4544
1 * tracer.activeSpan() >> span
@@ -63,7 +62,7 @@ class InsecureCookieModuleTest extends IastModuleImplTestBase {
6362
Vulnerability savedVul
6463

6564
when:
66-
module.onCookie(cookieName, false)
65+
module.onCookie(cookieName, cookieValue, isSecure, false, null)
6766

6867
then:
6968
1 * tracer.activeSpan() >> span
@@ -78,34 +77,16 @@ class InsecureCookieModuleTest extends IastModuleImplTestBase {
7877
}
7978

8079
where:
81-
cookieName | expected
82-
"user-id" | "user-id"
83-
}
84-
85-
void 'cases where nothing is reported during InsecureCookieModule.onCookie'() {
86-
87-
when:
88-
module.onCookie(cookieValue, isSecure)
89-
90-
then:
91-
0 * tracer.activeSpan()
92-
0 * overheadController._
93-
0 * reporter._
94-
95-
where:
96-
cookieValue | isSecure
97-
"user-id" | true
98-
null | true
99-
null | false
100-
"" | false
101-
"" | true
80+
cookieName | cookieValue | isSecure | expected
81+
"user-id" | "7" | false | "user-id"
10282
}
10383

104-
105-
void 'cases where nothing is reported during InsecureCookieModule.onCookieHeader'() {
84+
void 'cases where nothing is not reported during InsecureCookieModuleTest.onCookie'() {
85+
given:
86+
final cookie = HttpCookie.parse(cookieValue).first()
10687

10788
when:
108-
module.onCookieHeader(cookieValue)
89+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
10990

11091
then:
11192
0 * tracer.activeSpan()
@@ -114,30 +95,25 @@ class InsecureCookieModuleTest extends IastModuleImplTestBase {
11495

11596
where:
11697
cookieValue | _
117-
null | _
11898
"user-id=7; Secure" | _
119-
"user-id7; Secure" | _
12099
"user-id=7;Secure" | _
121-
"blah" | _
122-
"" | _
123100
}
124101

125-
void 'if onHeader receives a cookie header call onCookieHeader'(final String headerName, final int expected) {
126-
setup:
127-
final icm = Spy(InsecureCookieModuleImpl)
128-
InstrumentationBridge.registerIastModule(icm)
102+
void 'insecure cookie is not reported with InsecureCookieModule.onCookie'() {
103+
given:
104+
final cookie = new HttpCookie(cookieName, cookieValue)
105+
cookie.secure = isSecure
129106

130107
when:
131-
icm.onHeader(headerName, "value")
108+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
132109

133110
then:
134-
expected * icm.onCookieHeader("value")
135-
111+
0 * tracer.activeSpan() >> span
112+
0 * overheadController.consumeQuota(_, _) >> true
113+
0 * reporter.report(_, _ as Vulnerability)
136114

137115
where:
138-
headerName | expected
139-
"blah" | 0
140-
"Set-Cookie" | 1
141-
"set-cookie" | 1
116+
cookieName | cookieValue | isSecure
117+
"user-id" | "7" | true
142118
}
143119
}

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package com.datadog.iast.sink
2+
3+
import com.datadog.iast.IastModuleImplTestBase
4+
import com.datadog.iast.IastRequestContext
5+
import com.datadog.iast.model.Vulnerability
6+
import com.datadog.iast.model.VulnerabilityType
7+
import datadog.trace.api.gateway.RequestContext
8+
import datadog.trace.api.gateway.RequestContextSlot
9+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
10+
import groovy.transform.CompileDynamic
11+
12+
@CompileDynamic
13+
class NoHttpCookieModuleTest extends IastModuleImplTestBase {
14+
15+
private List<Object> objectHolder
16+
17+
private IastRequestContext ctx
18+
19+
private NoHttpOnlyCookieModuleImpl module
20+
21+
private AgentSpan span
22+
23+
def setup() {
24+
module = registerDependencies(new NoHttpOnlyCookieModuleImpl())
25+
objectHolder = []
26+
ctx = new IastRequestContext()
27+
final reqCtx = Mock(RequestContext) {
28+
getData(RequestContextSlot.IAST) >> ctx
29+
}
30+
span = Mock(AgentSpan) {
31+
getSpanId() >> 123456
32+
getRequestContext() >> reqCtx
33+
}
34+
}
35+
36+
void 'report NoHttp cookie with InsecureCookieModule.onCookie'() {
37+
given:
38+
Vulnerability savedVul
39+
final cookie = HttpCookie.parse(cookieValue).first()
40+
41+
when:
42+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
43+
44+
then:
45+
1 * tracer.activeSpan() >> span
46+
1 * overheadController.consumeQuota(_, _) >> true
47+
1 * reporter.report(_, _ as Vulnerability) >> { savedVul = it[1] }
48+
with(savedVul) {
49+
type == VulnerabilityType.NO_HTTPONLY_COOKIE
50+
location != null
51+
with(evidence) {
52+
value == expected
53+
}
54+
}
55+
56+
where:
57+
cookieValue | expected
58+
"user-id=7" | "user-id"
59+
}
60+
61+
void 'report insecure cookie with NoHttpOnlyCookieModule.onCookie'() {
62+
given:
63+
Vulnerability savedVul
64+
final cookie = new HttpCookie(cookieName, cookieValue)
65+
cookie.httpOnly = isHttpOnly
66+
67+
when:
68+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
69+
70+
then:
71+
1 * tracer.activeSpan() >> span
72+
1 * overheadController.consumeQuota(_, _) >> true
73+
1 * reporter.report(_, _ as Vulnerability) >> { savedVul = it[1] }
74+
with(savedVul) {
75+
type == VulnerabilityType.NO_HTTPONLY_COOKIE
76+
location != null
77+
with(evidence) {
78+
value == expected
79+
}
80+
}
81+
82+
where:
83+
cookieName | cookieValue | isHttpOnly | expected
84+
"user-id" | "7" | false | "user-id"
85+
}
86+
87+
void 'cases where nothing is reported during NoHttpModuleCookie.onCookie'() {
88+
given:
89+
final cookie = HttpCookie.parse(cookieValue).first()
90+
91+
when:
92+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
93+
94+
then:
95+
0 * tracer.activeSpan()
96+
0 * overheadController._
97+
0 * reporter._
98+
99+
where:
100+
cookieValue | _
101+
"user-id=7; HttpOnly" | _
102+
"user-id=7;HttpOnly" | _
103+
}
104+
105+
void 'insecure no http only is not reported with NoHttpOnlyCookieModule.onCookie'() {
106+
given:
107+
final cookie = new HttpCookie(cookieName, cookieValue)
108+
cookie.httpOnly = isHttpOnly
109+
110+
when:
111+
module.onCookie(cookie.name, cookie.value, cookie.secure, cookie.httpOnly, null)
112+
113+
then:
114+
0 * tracer.activeSpan() >> span
115+
0 * overheadController.consumeQuota(_, _) >> true
116+
0 * reporter.report(_, _ as Vulnerability)
117+
118+
where:
119+
cookieName | cookieValue | isHttpOnly
120+
"user-id" | "7" | true
121+
}
122+
}

‎dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JakartaWSResponseInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static void onExit(
4949
if (null != headerValue) {
5050
String value = headerValue.toString();
5151
if (value.length() > 0) {
52-
InstrumentationBridge.onHeader(headerName, value);
52+
InstrumentationBridge.RESPONSE_HEADER_MODULE.onHeader(headerName, value);
5353
}
5454
}
5555
}

‎dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JavaxWSResponseInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static void onExit(
4949
if (null != headerValue) {
5050
String value = headerValue.toString();
5151
if (value.length() > 0) {
52-
InstrumentationBridge.onHeader(headerName, value);
52+
InstrumentationBridge.RESPONSE_HEADER_MODULE.onHeader(headerName, value);
5353
}
5454
}
5555
}

0 commit comments

Comments
 (0)