Skip to content

Commit 531cdc8

Browse files
DDJavierSantosDDJavierSantos
committed
# This is a combination of 2 commits.
# This is the 1st commit message: started Ready for test checks spotless fixing tests Some more tests fixed spotless Some more fixes spotless Quota test added cleanup small optimization in HttpResponseHeaderModuleImpl optimization test fixes spotless # This is the commit message #2: fix a import missing from master rebase
1 parent a922167 commit 531cdc8

File tree

30 files changed

+524
-319
lines changed

30 files changed

+524
-319
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import com.datadog.iast.propagation.PropagationModuleImpl;
77
import com.datadog.iast.propagation.StringModuleImpl;
88
import com.datadog.iast.sink.CommandInjectionModuleImpl;
9+
import com.datadog.iast.sink.HttpResponseHeaderModuleImpl;
910
import com.datadog.iast.sink.InsecureCookieModuleImpl;
1011
import com.datadog.iast.sink.LdapInjectionModuleImpl;
1112
import com.datadog.iast.sink.NoHttpOnlyCookieModuleImpl;
13+
import com.datadog.iast.sink.NoSameSiteCookieModuleImpl;
1214
import com.datadog.iast.sink.PathTraversalModuleImpl;
1315
import com.datadog.iast.sink.SqlInjectionModuleImpl;
1416
import com.datadog.iast.sink.SsrfModuleImpl;
@@ -90,8 +92,10 @@ private static Stream<IastModule> iastModules() {
9092
new WeakHashModuleImpl(),
9193
new LdapInjectionModuleImpl(),
9294
new PropagationModuleImpl(),
95+
new HttpResponseHeaderModuleImpl(),
9396
new InsecureCookieModuleImpl(),
9497
new NoHttpOnlyCookieModuleImpl(),
98+
new NoSameSiteCookieModuleImpl(),
9599
new SsrfModuleImpl(),
96100
new UnvalidatedRedirectModuleImpl(),
97101
new WeakRandomnessModuleImpl());

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ public interface VulnerabilityType {
1111
VulnerabilityType INSECURE_COOKIE = new VulnerabilityTypeImpl(VulnerabilityTypes.INSECURE_COOKIE);
1212
VulnerabilityType NO_HTTPONLY_COOKIE =
1313
new VulnerabilityTypeImpl(VulnerabilityTypes.NO_HTTPONLY_COOKIE);
14+
VulnerabilityType NO_SAMESITE_COOKIE =
15+
new VulnerabilityTypeImpl(VulnerabilityTypes.NO_SAMESITE_COOKIE);
16+
1417
InjectionType SQL_INJECTION = new InjectionTypeImpl(VulnerabilityTypes.SQL_INJECTION, ' ');
1518
InjectionType COMMAND_INJECTION =
1619
new InjectionTypeImpl(VulnerabilityTypes.COMMAND_INJECTION, ' ');

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package com.datadog.iast.sink;
2+
3+
import com.datadog.iast.model.Evidence;
4+
import com.datadog.iast.model.Location;
5+
import com.datadog.iast.model.Vulnerability;
6+
import com.datadog.iast.model.VulnerabilityType;
7+
import com.datadog.iast.overhead.Operations;
8+
import com.datadog.iast.util.CookieSecurityDetails;
9+
import com.datadog.iast.util.CookieSecurityParser;
10+
import datadog.trace.api.iast.InstrumentationBridge;
11+
import datadog.trace.api.iast.sink.HttpResponseHeaderModule;
12+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
13+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
14+
import java.util.Arrays;
15+
import java.util.List;
16+
import javax.annotation.Nonnull;
17+
18+
public class HttpResponseHeaderModuleImpl extends SinkModuleBase
19+
implements HttpResponseHeaderModule {
20+
private static final String SET_COOKIE_HEADER = "Set-Cookie";
21+
22+
@Override
23+
public void onHeader(@Nonnull final String name, final String value) {
24+
if (SET_COOKIE_HEADER.equalsIgnoreCase(name)) {
25+
CookieSecurityParser cookieSecurityInfo = new CookieSecurityParser(value);
26+
onCookies(cookieSecurityInfo.getBadCookies());
27+
}
28+
if (null != InstrumentationBridge.UNVALIDATED_REDIRECT) {
29+
InstrumentationBridge.UNVALIDATED_REDIRECT.onHeader(name, value);
30+
}
31+
}
32+
33+
private void onCookies(List<CookieSecurityDetails> vulnerableCookies) {
34+
for (CookieSecurityDetails cookie : vulnerableCookies) {
35+
final AgentSpan span = AgentTracer.activeSpan();
36+
if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) {
37+
return;
38+
}
39+
Location location = Location.forSpanAndStack(spanId(span), getCurrentStackTrace());
40+
Evidence evidence = new Evidence(cookie.getCookieName());
41+
if (!cookie.isSecure() && null != InstrumentationBridge.INSECURE_COOKIE) {
42+
reporter.report(
43+
span, new Vulnerability(VulnerabilityType.INSECURE_COOKIE, location, evidence));
44+
}
45+
if (!cookie.isHttpOnly() && null != InstrumentationBridge.NO_HTTPONLY_COOKIE) {
46+
reporter.report(
47+
span, new Vulnerability(VulnerabilityType.NO_HTTPONLY_COOKIE, location, evidence));
48+
}
49+
if (!cookie.isSameSiteStrict() && null != InstrumentationBridge.NO_SAMESITE_COOKIE) {
50+
reporter.report(
51+
span, new Vulnerability(VulnerabilityType.NO_SAMESITE_COOKIE, location, evidence));
52+
}
53+
}
54+
}
55+
56+
@Override
57+
public void onCookie(
58+
@Nonnull final String name,
59+
final boolean isSecure,
60+
final boolean isHttpOnly,
61+
final boolean isSameSiteStrict) {
62+
CookieSecurityDetails details =
63+
new CookieSecurityDetails(name, isSecure, isHttpOnly, isSameSiteStrict);
64+
onCookies(Arrays.asList(details));
65+
}
66+
}

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

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,5 @@
11
package com.datadog.iast.sink;
22

3-
import com.datadog.iast.model.Evidence;
4-
import com.datadog.iast.model.VulnerabilityType;
5-
import com.datadog.iast.overhead.Operations;
63
import datadog.trace.api.iast.sink.InsecureCookieModule;
7-
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
8-
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
9-
import javax.annotation.Nonnull;
104

11-
public class InsecureCookieModuleImpl extends SinkModuleBase implements InsecureCookieModule {
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 (!isSecure) {
21-
final AgentSpan span = AgentTracer.activeSpan();
22-
if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) {
23-
return;
24-
}
25-
report(span, VulnerabilityType.INSECURE_COOKIE, new Evidence(name));
26-
}
27-
}
28-
}
5+
public class InsecureCookieModuleImpl extends SinkModuleBase implements InsecureCookieModule {}

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

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,5 @@
11
package com.datadog.iast.sink;
22

3-
import com.datadog.iast.model.Evidence;
4-
import com.datadog.iast.model.VulnerabilityType;
5-
import com.datadog.iast.overhead.Operations;
63
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;
104

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-
}
5+
public class NoHttpOnlyCookieModuleImpl extends SinkModuleBase implements NoHttpOnlyCookieModule {}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package com.datadog.iast.sink;
2+
3+
import datadog.trace.api.iast.sink.NoSameSiteCookieModule;
4+
5+
public class NoSameSiteCookieModuleImpl extends SinkModuleBase implements NoSameSiteCookieModule {}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ protected StackTraceElement getCurrentStackTrace() {
149149
return stackWalker.walk(SinkModuleBase::findValidPackageForVulnerability);
150150
}
151151

152-
private static long spanId(final AgentSpan span) {
152+
static long spanId(final AgentSpan span) {
153153
return span == null ? 0 : span.getSpanId();
154154
}
155155

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package com.datadog.iast.util;
2+
3+
public class CookieSecurityDetails {
4+
String cookieName;
5+
boolean isSecure = false;
6+
boolean isSameSiteStrict = false;
7+
boolean isHttpOnly = false;
8+
9+
public CookieSecurityDetails() {}
10+
11+
public CookieSecurityDetails(
12+
String cookieName, boolean isSecure, boolean isSameSiteStrict, boolean isHttpOnly) {
13+
this.cookieName = cookieName;
14+
this.isSecure = isSecure;
15+
this.isSameSiteStrict = isSameSiteStrict;
16+
this.isHttpOnly = isHttpOnly;
17+
}
18+
19+
void addAttribute(String name, String value) {
20+
if ("SECURE".equalsIgnoreCase(name) && null == value) {
21+
isSecure = true;
22+
}
23+
if ("HTTPONLY".equalsIgnoreCase(name) && "true".equalsIgnoreCase(value)) {
24+
isHttpOnly = true;
25+
}
26+
if ("SAMESITE".equalsIgnoreCase(name) && "strict".equalsIgnoreCase(value)) {
27+
isSameSiteStrict = true;
28+
}
29+
}
30+
31+
public String getCookieName() {
32+
return cookieName;
33+
}
34+
35+
public boolean isSecure() {
36+
return isSecure;
37+
}
38+
39+
public boolean isSameSiteStrict() {
40+
return isSameSiteStrict;
41+
}
42+
43+
public boolean isHttpOnly() {
44+
return isHttpOnly;
45+
}
46+
}

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package com.datadog.iast.util;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import java.util.NoSuchElementException;
6+
import java.util.StringTokenizer;
7+
8+
public class CookieSecurityParser {
9+
ArrayList<CookieSecurityDetails> badCookies = new ArrayList<>();
10+
11+
public CookieSecurityParser(String cookieString) {
12+
13+
List<CookieSecurityDetails> cookies = new java.util.ArrayList<>();
14+
15+
List<String> cookieStrings = splitMultiCookies(cookieString);
16+
for (String cookieStr : cookieStrings) {
17+
CookieSecurityDetails cookie = parseInternal(cookieStr);
18+
if (!cookie.isHttpOnly || !cookie.isSecure || !cookie.isSameSiteStrict) {
19+
badCookies.add(cookie);
20+
}
21+
}
22+
}
23+
24+
public ArrayList<CookieSecurityDetails> getBadCookies() {
25+
return badCookies;
26+
}
27+
28+
private CookieSecurityDetails parseInternal(String header) {
29+
CookieSecurityDetails analyzedCookie = new CookieSecurityDetails();
30+
31+
StringTokenizer tokenizer = new StringTokenizer(header, ";");
32+
33+
// there should always have at least on name-value pair;
34+
// it's cookie's name
35+
try {
36+
String nameValuePair = tokenizer.nextToken();
37+
int index = nameValuePair.indexOf('=');
38+
if (index != -1) {
39+
analyzedCookie.cookieName = nameValuePair.substring(0, index).trim();
40+
} else {
41+
return null;
42+
}
43+
} catch (NoSuchElementException ignored) {
44+
return null;
45+
}
46+
47+
// remaining name-value pairs are cookie's attributes
48+
while (tokenizer.hasMoreTokens()) {
49+
String attribute = tokenizer.nextToken();
50+
int index = attribute.indexOf('=');
51+
String name, value;
52+
if (index != -1) {
53+
name = attribute.substring(0, index).trim();
54+
value = attribute.substring(index + 1).trim();
55+
} else {
56+
name = attribute.trim();
57+
value = null;
58+
}
59+
analyzedCookie.addAttribute(name, value);
60+
}
61+
62+
return analyzedCookie;
63+
}
64+
65+
private static List<String> splitMultiCookies(String header) {
66+
List<String> cookies = new java.util.ArrayList<String>();
67+
int quoteCount = 0;
68+
int p, q;
69+
70+
for (p = 0, q = 0; p < header.length(); p++) {
71+
char c = header.charAt(p);
72+
if (c == '"') quoteCount++;
73+
if (c == ',' && (quoteCount % 2 == 0)) {
74+
// it is comma and not surrounding by double-quotes
75+
cookies.add(header.substring(q, p));
76+
q = p + 1;
77+
}
78+
}
79+
80+
cookies.add(header.substring(q));
81+
82+
return cookies;
83+
}
84+
}

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
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.api.iast.InstrumentationBridge
10+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
11+
12+
class HttpResponseHeaderModuleTest extends IastModuleImplTestBase {
13+
14+
private List<Object> objectHolder
15+
16+
private IastRequestContext ctx
17+
18+
private HttpResponseHeaderModuleImpl module
19+
20+
private AgentSpan span
21+
22+
def setup() {
23+
InstrumentationBridge.clearIastModules()
24+
module = registerDependencies(new HttpResponseHeaderModuleImpl())
25+
InstrumentationBridge.registerIastModule(new InsecureCookieModuleImpl())
26+
InstrumentationBridge.registerIastModule(new NoHttpOnlyCookieModuleImpl())
27+
InstrumentationBridge.registerIastModule(new NoSameSiteCookieModuleImpl())
28+
objectHolder = []
29+
ctx = new IastRequestContext()
30+
final reqCtx = Mock(RequestContext) {
31+
getData(RequestContextSlot.IAST) >> ctx
32+
}
33+
span = Mock(AgentSpan) {
34+
getSpanId() >> 123456
35+
getRequestContext() >> reqCtx
36+
}
37+
}
38+
39+
40+
void 'check quota is consumed correctly'() {
41+
given:
42+
Vulnerability savedVul1
43+
Vulnerability savedVul2
44+
Vulnerability savedVul3
45+
46+
when:
47+
module.onCookie("user-id", false, false, false)
48+
49+
then:
50+
1 * tracer.activeSpan() >> span
51+
4 * span.getSpanId()
52+
1 * overheadController.consumeQuota(_, _) >> true
53+
1 * reporter.report(_, _ as Vulnerability) >> { savedVul1 = it[1] }
54+
1 * reporter.report(_, _ as Vulnerability) >> { savedVul2 = it[1] }
55+
1 * reporter.report(_, _ as Vulnerability) >> { savedVul3 = it[1] }
56+
0 * _
57+
with(savedVul1) {
58+
type == VulnerabilityType.INSECURE_COOKIE
59+
location != null
60+
with(evidence) {
61+
value == "user-id"
62+
}
63+
}
64+
with(savedVul2) {
65+
type == VulnerabilityType.NO_HTTPONLY_COOKIE
66+
location != null
67+
with(evidence) {
68+
value == "user-id"
69+
}
70+
}
71+
with(savedVul3) {
72+
type == VulnerabilityType.NO_SAMESITE_COOKIE
73+
location != null
74+
with(evidence) {
75+
value == "user-id"
76+
}
77+
}
78+
}
79+
80+
void 'exercise onHeader'() {
81+
when:
82+
module.onHeader("Set-Cookie", "user-id=7")
83+
84+
then:
85+
1 * tracer.activeSpan()
86+
0 * _
87+
}
88+
}

0 commit comments

Comments
 (0)