Skip to content

Commit 4c97fc1

Browse files
authored
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
1 parent b075ba5 commit 4c97fc1

5 files changed

Lines changed: 52 additions & 3 deletions

File tree

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ public HashBasedDeduplication(@Nullable final AgentTaskScheduler taskScheduler)
152152

153153
@Override
154154
public boolean test(final Vulnerability vulnerability) {
155+
if (!vulnerability.getType().isDeduplicable()) {
156+
return false;
157+
}
155158
final boolean newVulnerability = hashes.add(vulnerability.getHash());
156159
if (newVulnerability && hashes.size() > maxSize) {
157160
hashes.clear();

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public interface VulnerabilityType {
8787
VulnerabilityTypes.REFLECTION_INJECTION, VulnerabilityMarks.REFLECTION_INJECTION_MARK);
8888

8989
VulnerabilityType SESSION_REWRITING =
90-
new ServiceVulnerabilityType(VulnerabilityTypes.SESSION_REWRITING);
90+
new ServiceVulnerabilityType(VulnerabilityTypes.SESSION_REWRITING, false);
9191

9292
String name();
9393

@@ -98,6 +98,9 @@ public interface VulnerabilityType {
9898

9999
long calculateHash(@Nonnull final Vulnerability vulnerability);
100100

101+
/** A flag to indicate if the vulnerability is deduplicable. */
102+
boolean isDeduplicable();
103+
101104
class VulnerabilityTypeImpl implements VulnerabilityType {
102105

103106
private final byte type;
@@ -106,14 +109,26 @@ class VulnerabilityTypeImpl implements VulnerabilityType {
106109

107110
private final int mark;
108111

112+
private final boolean deduplicable;
113+
109114
public VulnerabilityTypeImpl(final byte type, final int... marks) {
110115
this(type, ' ', marks);
111116
}
112117

118+
public VulnerabilityTypeImpl(final byte type, boolean deduplicable, final int... marks) {
119+
this(type, ' ', deduplicable, marks);
120+
}
121+
113122
public VulnerabilityTypeImpl(final byte type, final char separator, final int... marks) {
123+
this(type, separator, true, marks);
124+
}
125+
126+
public VulnerabilityTypeImpl(
127+
final byte type, final char separator, final boolean deduplicable, final int... marks) {
114128
this.type = type;
115129
this.separator = separator;
116130
mark = computeMarks(marks);
131+
this.deduplicable = deduplicable;
117132
}
118133

119134
@Override
@@ -148,6 +163,11 @@ public long calculateHash(@Nonnull final Vulnerability vulnerability) {
148163
return crc.getValue();
149164
}
150165

166+
@Override
167+
public boolean isDeduplicable() {
168+
return deduplicable;
169+
}
170+
151171
protected void update(final CRC32 crc, final String value) {
152172
final byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
153173
crc.update(bytes, 0, bytes.length);
@@ -197,8 +217,8 @@ public long calculateHash(@Nonnull final Vulnerability vulnerability) {
197217
}
198218

199219
class ServiceVulnerabilityType extends VulnerabilityTypeImpl {
200-
public ServiceVulnerabilityType(byte type, int... marks) {
201-
super(type, marks);
220+
public ServiceVulnerabilityType(byte type, boolean deduplicable, int... marks) {
221+
super(type, deduplicable, marks);
202222
}
203223

204224
@Override

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public void checkSessionTrackingModes(@Nonnull Set<String> sessionTrackingModes)
112112
}
113113
final AgentSpan span = AgentTracer.activeSpan();
114114
// overhead is not checked here as it's called once per application context
115+
// No deduplication is needed as same service can have multiple applications
115116
reporter.report(
116117
span,
117118
new Vulnerability(

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,30 @@ class ReporterTest extends DDSpecification {
449449
0 * _
450450
}
451451

452+
void 'Reporter when vulnerability is no deduplicable does not prevent duplicates'() {
453+
given:
454+
final Reporter reporter = new Reporter()
455+
final batch = new VulnerabilityBatch()
456+
final span = spanWithBatch(batch)
457+
final vulnerability = new Vulnerability(
458+
VulnerabilityType.SESSION_REWRITING,
459+
Location.forSpan(span),
460+
new Evidence("SESSION_REWRITING")
461+
)
462+
463+
when: 'first time a vulnerability is reported'
464+
reporter.report(span, vulnerability)
465+
466+
then:
467+
batch.vulnerabilities.size() == 1
468+
469+
when: 'second time the a vulnerability is reported'
470+
reporter.report(span, vulnerability)
471+
472+
then:
473+
batch.vulnerabilities.size() == 2
474+
}
475+
452476
private AgentSpan spanWithBatch(final VulnerabilityBatch batch) {
453477
final traceSegment = Mock(TraceSegment) {
454478
getDataTop('iast') >> batch

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class ApplicationModuleTest extends IastModuleImplTestBase {
8484
} else {
8585
0 * reporter.report(_, _)
8686
}
87+
0 * reporter.report(_, _)
8788

8889
where:
8990
sessionTrackingModes | expected

0 commit comments

Comments
 (0)