diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java index f9a05fec280..04ebec93407 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java @@ -137,6 +137,8 @@ public interface VulnerabilityType { /** A flag to indicate if the vulnerability is deduplicable. */ boolean isDeduplicable(); + byte type(); + static Builder type(final byte type) { return new Builder(type); } @@ -191,6 +193,11 @@ public boolean isDeduplicable() { return deduplicable; } + @Override + public byte type() { + return type; + } + /** Useful for troubleshooting issues when vulns are serialized without moshi */ public String getName() { return name(); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java index a735ebd4a13..45938cc72c4 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java @@ -2,6 +2,7 @@ import static com.datadog.iast.util.ObjectVisitor.State.CONTINUE; import static com.datadog.iast.util.ObjectVisitor.State.EXIT; +import static datadog.trace.api.iast.VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK; import com.datadog.iast.Dependencies; import com.datadog.iast.Reporter; @@ -22,6 +23,8 @@ import datadog.trace.api.Pair; import datadog.trace.api.iast.IastContext; import datadog.trace.api.iast.Taintable; +import datadog.trace.api.iast.telemetry.IastMetric; +import datadog.trace.api.iast.telemetry.IastMetricCollector; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.instrumentation.iastinstrumenter.IastExclusionTrie; @@ -126,6 +129,7 @@ protected final Evidence checkInjection( final TaintedObject tainted = origin == null ? null : to.get(origin); if (origin != null && tainted != null) { valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark()); + addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type); value = origin; } else { valueRanges = Ranges.forObject((Source) taintable.$$DD$getSource(), type.mark()); @@ -137,6 +141,7 @@ protected final Evidence checkInjection( return null; } valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark()); + addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type); } if (valueRanges == null || valueRanges.length == 0) { @@ -160,6 +165,25 @@ protected final Evidence checkInjection( return report(span, type, evidence, ranges, locationSupplier); } + private void addSecurityControlMetrics( + @Nonnull final IastContext ctx, + @Nullable final Range[] valueRanges, + @Nonnull final Range[] taintedRanges, + @Nonnull final VulnerabilityType type) { + if ((valueRanges != null + && valueRanges.length + != 0) // ranges without the vulnerability mark implies vulnerability + || taintedRanges.length == 0 // no tainted ranges + ) { + return; + } + // check if there are tainted ranges without the security control mark + Range[] marked = Ranges.getNotMarkedRanges(taintedRanges, CUSTOM_SECURITY_CONTROL_MARK); + if (marked == null || marked.length == 0) { + IastMetricCollector.add(IastMetric.SUPPRESSED_VULNERABILITIES, type.type(), 1, ctx); + } + } + @Nullable protected final Evidence checkInjection(final VulnerabilityType type, final Iterator items) { return checkInjection(type, items, null, null); @@ -207,6 +231,7 @@ protected final Evidence checkInjection( Range[] valueRanges = null; if (tainted != null) { valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark()); + addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type); } addToEvidence(type, evidence, ranges, value, valueRanges, evidenceBuilder); diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy index d87583eabd4..67866d2eb21 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy @@ -1,18 +1,24 @@ package com.datadog.iast.securitycontrol +import com.datadog.iast.IastModuleImplTestBase +import com.datadog.iast.sink.XssModuleImpl +import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.VulnerabilityMarks +import static datadog.trace.api.iast.VulnerabilityMarks.NOT_MARKED +import datadog.trace.api.iast.VulnerabilityTypes import datadog.trace.api.iast.propagation.PropagationModule import datadog.trace.api.iast.securitycontrol.SecurityControl import datadog.trace.api.iast.securitycontrol.SecurityControlFormatter -import datadog.trace.test.util.DDSpecification +import static datadog.trace.api.iast.telemetry.IastMetric.SUPPRESSED_VULNERABILITIES +import datadog.trace.api.iast.telemetry.IastMetricCollector import foo.bar.securitycontrol.SecurityControlStaticTestSuite import foo.bar.securitycontrol.SecurityControlTestSuite import net.bytebuddy.agent.ByteBuddyAgent import java.lang.instrument.Instrumentation -class IastSecurityControlTransformerForkedTest extends DDSpecification{ +class IastSecurityControlTransformerForkedTest extends IastModuleImplTestBase{ //static methods private static final String STATIC_SANITIZER = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitize' @@ -43,6 +49,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ private static final String INPUT_VALIDATOR_VALIDATE_SELECTED_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateSelectedLong:0' + private IastMetricCollector mockCollector def setupSpec() { 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,18 +62,23 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ instrumentation.addTransformer(new IastSecurityControlTransformer(securityControls), true) } + void setup() { + mockCollector = Mock(IastMetricCollector) + ctx.collector = mockCollector + } + void 'test sanitize'(){ given: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) + final propagationModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(propagationModule) final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) when: SecurityControlStaticTestSuite.&"$method".call(*args) then: - expected * iastModule.markIfTainted( toSanitize, marks) + expected * propagationModule.markIfTainted( toSanitize, marks) 0 * _ when: @@ -74,7 +86,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ suite.&"$method".call(*args) then: - expected * iastModule.markIfTainted( toSanitize, marks) + expected * propagationModule.markIfTainted( toSanitize, marks) 0 * _ where: @@ -89,8 +101,8 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ void 'test validate'(){ given: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) + final propagationModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(propagationModule) final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) when: @@ -98,7 +110,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ then: for (final validate : toValidate){ - expected * iastModule.markIfTainted(validate, marks) + expected * propagationModule.markIfTainted(validate, marks) } 0 * _ @@ -108,7 +120,7 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ then: for (final validate : toValidate){ - expected * iastModule.markIfTainted(validate, marks) + expected * propagationModule.markIfTainted(validate, marks) } 0 * _ @@ -128,4 +140,36 @@ class IastSecurityControlTransformerForkedTest extends DDSpecification{ 'validateSelectedLong' | [1L] | args | 0 'validateSelectedLong' | [1L, 2L] | [args[0]] | 0 } + + void 'test metrics'() { + setup: + final iastModule = new XssModuleImpl(dependencies) + final param = mapTainted(s, mark) + + when: + iastModule.onXss(param as String) + + then: + expected * mockCollector.addMetric(SUPPRESSED_VULNERABILITIES, VulnerabilityTypes.XSS, 1) + + where: + s | mark | expected + null | NOT_MARKED | 0 + '/var' | NOT_MARKED | 0 + '/==>var<==' | NOT_MARKED | 0 + '/==>var<==' | VulnerabilityMarks.XSS_MARK | 0 + '/==>var<==' | VulnerabilityMarks.SQL_INJECTION_MARK | 0 + '/==>var<==' | combine(VulnerabilityMarks.SQL_INJECTION_MARK, VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) | 0 + '/==>var<==' | combine(VulnerabilityMarks.XSS_MARK, VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) | 1 + } + + private static int combine(int mark1, int mark2) { + return mark1 | mark2 // Perform the bitwise OR + } + + private String mapTainted(final String value, final int mark) { + final result = addFromTaintFormat(ctx.taintedObjects, value, mark) + objectHolder.add(result) + return result + } } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java index 42dd7e91c4d..a986535f0bc 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java @@ -27,7 +27,6 @@ public final class IastConfig { public static final String IAST_EXPERIMENTAL_PROPAGATION_ENABLED = "iast.experimental.propagation.enabled"; public static final String IAST_STACK_TRACE_ENABLED = "iast.stacktrace.enabled"; - public static final String IAST_SECURITY_CONTROLS_ENABLED = "iast.security-controls.enabled"; public static final String IAST_SECURITY_CONTROLS_CONFIGURATION = "iast.security-controls.configuration"; diff --git a/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java b/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java index cfe18789e65..736ed126146 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java @@ -22,6 +22,12 @@ public enum IastMetric { JSON_TAG_SIZE_EXCEED("json.tag.size.exceeded", true, Scope.GLOBAL, Verbosity.INFORMATION), SOURCE_MAPPING_LIMIT_REACHED( "source.mapping.limit.reached", true, Scope.GLOBAL, Verbosity.INFORMATION), + SUPPRESSED_VULNERABILITIES( + "suppressed.vulnerabilities", + true, + Scope.REQUEST, + Tag.VULNERABILITY_TYPE, + Verbosity.INFORMATION), EXPERIMENTAL_PROPAGATION("experimental.propagation", false, Scope.GLOBAL, Verbosity.INFORMATION); private static final int COUNT;