From 4c97fc1a28e22430680943973d8ac8fb750d06ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Tue, 16 Apr 2024 08:21:05 +0200 Subject: [PATCH] Remove deduplication for session rewriting vulnerability report (#6895) ##What Does This Do Add a new method Reporter#noDepupReport to allow report vulnerabilities without deduplication Remove deduplication for session rewriting vulnerability report ##Motivation If several apps are deployed in the same server only the first one session rewriting vulnerability find will be reported --- .../main/java/com/datadog/iast/Reporter.java | 3 +++ .../datadog/iast/model/VulnerabilityType.java | 26 ++++++++++++++++--- .../iast/sink/ApplicationModuleImpl.java | 1 + .../com/datadog/iast/ReporterTest.groovy | 24 +++++++++++++++++ .../iast/sink/ApplicationModuleTest.groovy | 1 + 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java index c498687431f..5d6d31c26b4 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java @@ -152,6 +152,9 @@ public HashBasedDeduplication(@Nullable final AgentTaskScheduler taskScheduler) @Override public boolean test(final Vulnerability vulnerability) { + if (!vulnerability.getType().isDeduplicable()) { + return false; + } final boolean newVulnerability = hashes.add(vulnerability.getHash()); if (newVulnerability && hashes.size() > maxSize) { hashes.clear(); 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 8b7810f69c7..8e484bee341 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 @@ -87,7 +87,7 @@ public interface VulnerabilityType { VulnerabilityTypes.REFLECTION_INJECTION, VulnerabilityMarks.REFLECTION_INJECTION_MARK); VulnerabilityType SESSION_REWRITING = - new ServiceVulnerabilityType(VulnerabilityTypes.SESSION_REWRITING); + new ServiceVulnerabilityType(VulnerabilityTypes.SESSION_REWRITING, false); String name(); @@ -98,6 +98,9 @@ public interface VulnerabilityType { long calculateHash(@Nonnull final Vulnerability vulnerability); + /** A flag to indicate if the vulnerability is deduplicable. */ + boolean isDeduplicable(); + class VulnerabilityTypeImpl implements VulnerabilityType { private final byte type; @@ -106,14 +109,26 @@ class VulnerabilityTypeImpl implements VulnerabilityType { private final int mark; + private final boolean deduplicable; + public VulnerabilityTypeImpl(final byte type, final int... marks) { this(type, ' ', marks); } + public VulnerabilityTypeImpl(final byte type, boolean deduplicable, final int... marks) { + this(type, ' ', deduplicable, marks); + } + public VulnerabilityTypeImpl(final byte type, final char separator, final int... marks) { + this(type, separator, true, marks); + } + + public VulnerabilityTypeImpl( + final byte type, final char separator, final boolean deduplicable, final int... marks) { this.type = type; this.separator = separator; mark = computeMarks(marks); + this.deduplicable = deduplicable; } @Override @@ -148,6 +163,11 @@ public long calculateHash(@Nonnull final Vulnerability vulnerability) { return crc.getValue(); } + @Override + public boolean isDeduplicable() { + return deduplicable; + } + protected void update(final CRC32 crc, final String value) { final byte[] bytes = value.getBytes(StandardCharsets.UTF_8); crc.update(bytes, 0, bytes.length); @@ -197,8 +217,8 @@ public long calculateHash(@Nonnull final Vulnerability vulnerability) { } class ServiceVulnerabilityType extends VulnerabilityTypeImpl { - public ServiceVulnerabilityType(byte type, int... marks) { - super(type, marks); + public ServiceVulnerabilityType(byte type, boolean deduplicable, int... marks) { + super(type, deduplicable, marks); } @Override diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java index e653a758b9f..d3357a88d81 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java @@ -112,6 +112,7 @@ public void checkSessionTrackingModes(@Nonnull Set sessionTrackingModes) } final AgentSpan span = AgentTracer.activeSpan(); // overhead is not checked here as it's called once per application context + // No deduplication is needed as same service can have multiple applications reporter.report( span, new Vulnerability( diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy index 80e02622ae1..662e1105f9b 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy @@ -449,6 +449,30 @@ class ReporterTest extends DDSpecification { 0 * _ } + void 'Reporter when vulnerability is no deduplicable does not prevent duplicates'() { + given: + final Reporter reporter = new Reporter() + final batch = new VulnerabilityBatch() + final span = spanWithBatch(batch) + final vulnerability = new Vulnerability( + VulnerabilityType.SESSION_REWRITING, + Location.forSpan(span), + new Evidence("SESSION_REWRITING") + ) + + when: 'first time a vulnerability is reported' + reporter.report(span, vulnerability) + + then: + batch.vulnerabilities.size() == 1 + + when: 'second time the a vulnerability is reported' + reporter.report(span, vulnerability) + + then: + batch.vulnerabilities.size() == 2 + } + private AgentSpan spanWithBatch(final VulnerabilityBatch batch) { final traceSegment = Mock(TraceSegment) { getDataTop('iast') >> batch diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy index 32b0947ce20..4cf286bff48 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy @@ -84,6 +84,7 @@ class ApplicationModuleTest extends IastModuleImplTestBase { } else { 0 * reporter.report(_, _) } + 0 * reporter.report(_, _) where: sessionTrackingModes | expected