Skip to content

Commit d402356

Browse files
jandro996jandro996
authored
Add security control metrics (#8175)
What Does This Do Add suppressed.vulnerabilities metrics when a vulnerability is suppressed due to a security control - RFC (Milestone 1) Additional Notes When checking if a vulnerability has occurred, if there are no ranges without the mark for the vulnerability being detected, and all ranges are marked with the CUSTOM_SECURE_MARK, we will send the metric.
1 parent 9b8eee7 commit d402356

File tree

5 files changed

+92
-11
lines changed

5 files changed

+92
-11
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ public interface VulnerabilityType {
137137
/** A flag to indicate if the vulnerability is deduplicable. */
138138
boolean isDeduplicable();
139139

140+
byte type();
141+
140142
static Builder type(final byte type) {
141143
return new Builder(type);
142144
}
@@ -191,6 +193,11 @@ public boolean isDeduplicable() {
191193
return deduplicable;
192194
}
193195

196+
@Override
197+
public byte type() {
198+
return type;
199+
}
200+
194201
/** Useful for troubleshooting issues when vulns are serialized without moshi */
195202
public String getName() {
196203
return name();

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static com.datadog.iast.util.ObjectVisitor.State.CONTINUE;
44
import static com.datadog.iast.util.ObjectVisitor.State.EXIT;
5+
import static datadog.trace.api.iast.VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK;
56

67
import com.datadog.iast.Dependencies;
78
import com.datadog.iast.Reporter;
@@ -22,6 +23,8 @@
2223
import datadog.trace.api.Pair;
2324
import datadog.trace.api.iast.IastContext;
2425
import datadog.trace.api.iast.Taintable;
26+
import datadog.trace.api.iast.telemetry.IastMetric;
27+
import datadog.trace.api.iast.telemetry.IastMetricCollector;
2528
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
2629
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
2730
import datadog.trace.instrumentation.iastinstrumenter.IastExclusionTrie;
@@ -126,6 +129,7 @@ protected final Evidence checkInjection(
126129
final TaintedObject tainted = origin == null ? null : to.get(origin);
127130
if (origin != null && tainted != null) {
128131
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
132+
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
129133
value = origin;
130134
} else {
131135
valueRanges = Ranges.forObject((Source) taintable.$$DD$getSource(), type.mark());
@@ -137,6 +141,7 @@ protected final Evidence checkInjection(
137141
return null;
138142
}
139143
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
144+
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
140145
}
141146

142147
if (valueRanges == null || valueRanges.length == 0) {
@@ -160,6 +165,25 @@ protected final Evidence checkInjection(
160165
return report(span, type, evidence, ranges, locationSupplier);
161166
}
162167

168+
private void addSecurityControlMetrics(
169+
@Nonnull final IastContext ctx,
170+
@Nullable final Range[] valueRanges,
171+
@Nonnull final Range[] taintedRanges,
172+
@Nonnull final VulnerabilityType type) {
173+
if ((valueRanges != null
174+
&& valueRanges.length
175+
!= 0) // ranges without the vulnerability mark implies vulnerability
176+
|| taintedRanges.length == 0 // no tainted ranges
177+
) {
178+
return;
179+
}
180+
// check if there are tainted ranges without the security control mark
181+
Range[] marked = Ranges.getNotMarkedRanges(taintedRanges, CUSTOM_SECURITY_CONTROL_MARK);
182+
if (marked == null || marked.length == 0) {
183+
IastMetricCollector.add(IastMetric.SUPPRESSED_VULNERABILITIES, type.type(), 1, ctx);
184+
}
185+
}
186+
163187
@Nullable
164188
protected final Evidence checkInjection(final VulnerabilityType type, final Iterator<?> items) {
165189
return checkInjection(type, items, null, null);
@@ -207,6 +231,7 @@ protected final Evidence checkInjection(
207231
Range[] valueRanges = null;
208232
if (tainted != null) {
209233
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
234+
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
210235
}
211236
addToEvidence(type, evidence, ranges, value, valueRanges, evidenceBuilder);
212237

‎dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy‎

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
11
package com.datadog.iast.securitycontrol
22

3+
import com.datadog.iast.IastModuleImplTestBase
4+
import com.datadog.iast.sink.XssModuleImpl
5+
import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat
36
import datadog.trace.api.iast.InstrumentationBridge
47
import datadog.trace.api.iast.VulnerabilityMarks
8+
import static datadog.trace.api.iast.VulnerabilityMarks.NOT_MARKED
9+
import datadog.trace.api.iast.VulnerabilityTypes
510
import datadog.trace.api.iast.propagation.PropagationModule
611
import datadog.trace.api.iast.securitycontrol.SecurityControl
712
import datadog.trace.api.iast.securitycontrol.SecurityControlFormatter
8-
import datadog.trace.test.util.DDSpecification
13+
import static datadog.trace.api.iast.telemetry.IastMetric.SUPPRESSED_VULNERABILITIES
14+
import datadog.trace.api.iast.telemetry.IastMetricCollector
915
import foo.bar.securitycontrol.SecurityControlStaticTestSuite
1016
import foo.bar.securitycontrol.SecurityControlTestSuite
1117
import net.bytebuddy.agent.ByteBuddyAgent
1218

1319
import java.lang.instrument.Instrumentation
1420

15-
class IastSecurityControlTransformerForkedTest extends DDSpecification{
21+
class IastSecurityControlTransformerForkedTest extends IastModuleImplTestBase{
1622

1723
//static methods
1824
private static final String STATIC_SANITIZER = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitize'
@@ -43,6 +49,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{
4349
private static final String INPUT_VALIDATOR_VALIDATE_SELECTED_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateSelectedLong:0'
4450

4551

52+
private IastMetricCollector mockCollector
4653

4754
def setupSpec() {
4855
final staticConfig = "${STATIC_SANITIZER};${STATIC_SANITIZE_VALIDATE_OBJECT};${STATIC_SANITIZE_INPUTS};${STATIC_SANITIZE_MANY_INPUTS};${STATIC_SANITIZE_INT};${STATIC_SANITIZE_LONG};${STATIC_INPUT_VALIDATOR_VALIDATE_ALL};${STATIC_INPUT_VALIDATOR_VALIDATE_OVERLOADED};${STATIC_INPUT_VALIDATOR_VALIDATE_RETURNING_INT};${STATIC_INPUT_VALIDATOR_VALIDATE_OBJECT};${STATIC_INPUT_VALIDATOR_VALIDATE_LONG};${STATIC_INPUT_VALIDATOR_VALIDATE_SELECTED_LONG}"
@@ -55,26 +62,31 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{
5562
instrumentation.addTransformer(new IastSecurityControlTransformer(securityControls), true)
5663
}
5764

65+
void setup() {
66+
mockCollector = Mock(IastMetricCollector)
67+
ctx.collector = mockCollector
68+
}
69+
5870

5971
void 'test sanitize'(){
6072
given:
61-
final iastModule = Mock(PropagationModule)
62-
InstrumentationBridge.registerIastModule(iastModule)
73+
final propagationModule = Mock(PropagationModule)
74+
InstrumentationBridge.registerIastModule(propagationModule)
6375
final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK)
6476

6577
when:
6678
SecurityControlStaticTestSuite.&"$method".call(*args)
6779

6880
then:
69-
expected * iastModule.markIfTainted( toSanitize, marks)
81+
expected * propagationModule.markIfTainted( toSanitize, marks)
7082
0 * _
7183

7284
when:
7385
final suite = new SecurityControlTestSuite()
7486
suite.&"$method".call(*args)
7587

7688
then:
77-
expected * iastModule.markIfTainted( toSanitize, marks)
89+
expected * propagationModule.markIfTainted( toSanitize, marks)
7890
0 * _
7991

8092
where:
@@ -89,16 +101,16 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{
89101

90102
void 'test validate'(){
91103
given:
92-
final iastModule = Mock(PropagationModule)
93-
InstrumentationBridge.registerIastModule(iastModule)
104+
final propagationModule = Mock(PropagationModule)
105+
InstrumentationBridge.registerIastModule(propagationModule)
94106
final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK)
95107

96108
when:
97109
SecurityControlStaticTestSuite.&"$method".call(*args)
98110

99111
then:
100112
for (final validate : toValidate){
101-
expected * iastModule.markIfTainted(validate, marks)
113+
expected * propagationModule.markIfTainted(validate, marks)
102114
}
103115
0 * _
104116

@@ -108,7 +120,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{
108120

109121
then:
110122
for (final validate : toValidate){
111-
expected * iastModule.markIfTainted(validate, marks)
123+
expected * propagationModule.markIfTainted(validate, marks)
112124
}
113125
0 * _
114126

@@ -128,4 +140,36 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{
128140
'validateSelectedLong' | [1L] | args | 0
129141
'validateSelectedLong' | [1L, 2L] | [args[0]] | 0
130142
}
143+
144+
void 'test metrics'() {
145+
setup:
146+
final iastModule = new XssModuleImpl(dependencies)
147+
final param = mapTainted(s, mark)
148+
149+
when:
150+
iastModule.onXss(param as String)
151+
152+
then:
153+
expected * mockCollector.addMetric(SUPPRESSED_VULNERABILITIES, VulnerabilityTypes.XSS, 1)
154+
155+
where:
156+
s | mark | expected
157+
null | NOT_MARKED | 0
158+
'/var' | NOT_MARKED | 0
159+
'/==>var<==' | NOT_MARKED | 0
160+
'/==>var<==' | VulnerabilityMarks.XSS_MARK | 0
161+
'/==>var<==' | VulnerabilityMarks.SQL_INJECTION_MARK | 0
162+
'/==>var<==' | combine(VulnerabilityMarks.SQL_INJECTION_MARK, VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) | 0
163+
'/==>var<==' | combine(VulnerabilityMarks.XSS_MARK, VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) | 1
164+
}
165+
166+
private static int combine(int mark1, int mark2) {
167+
return mark1 | mark2 // Perform the bitwise OR
168+
}
169+
170+
private String mapTainted(final String value, final int mark) {
171+
final result = addFromTaintFormat(ctx.taintedObjects, value, mark)
172+
objectHolder.add(result)
173+
return result
174+
}
131175
}

‎dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ public final class IastConfig {
2727
public static final String IAST_EXPERIMENTAL_PROPAGATION_ENABLED =
2828
"iast.experimental.propagation.enabled";
2929
public static final String IAST_STACK_TRACE_ENABLED = "iast.stacktrace.enabled";
30-
public static final String IAST_SECURITY_CONTROLS_ENABLED = "iast.security-controls.enabled";
3130
public static final String IAST_SECURITY_CONTROLS_CONFIGURATION =
3231
"iast.security-controls.configuration";
3332

‎internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ public enum IastMetric {
2222
JSON_TAG_SIZE_EXCEED("json.tag.size.exceeded", true, Scope.GLOBAL, Verbosity.INFORMATION),
2323
SOURCE_MAPPING_LIMIT_REACHED(
2424
"source.mapping.limit.reached", true, Scope.GLOBAL, Verbosity.INFORMATION),
25+
SUPPRESSED_VULNERABILITIES(
26+
"suppressed.vulnerabilities",
27+
true,
28+
Scope.REQUEST,
29+
Tag.VULNERABILITY_TYPE,
30+
Verbosity.INFORMATION),
2531
EXPERIMENTAL_PROPAGATION("experimental.propagation", false, Scope.GLOBAL, Verbosity.INFORMATION);
2632

2733
private static final int COUNT;

0 commit comments

Comments
 (0)