From 3404e3e6dc24d2aac18e73c5c3ddbe0d7e587bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 31 Oct 2023 13:57:15 +0100 Subject: [PATCH] Try to match tainted objects with sources when checking vulnerabilities with unbounded objects --- .../com/datadog/iast/sink/SinkModuleBase.java | 24 +++++++- .../java/com/datadog/iast/taint/Ranges.java | 8 +++ .../iast/sink/AbstractSinkModuleTest.groovy | 57 ++++++++++++++++++- 3 files changed, 85 insertions(+), 4 deletions(-) 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 d1a24a5516f..dce22f3a27d 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.NOT_MARKED; import com.datadog.iast.Dependencies; import com.datadog.iast.IastRequestContext; @@ -57,7 +58,7 @@ protected SinkModuleBase(@Nonnull final Dependencies dependencies) { if (!overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span)) { return null; } - final Evidence result = new Evidence(value.toString(), ranges); + final Evidence result = buildEvidence(value, ranges); report(span, type, result); return result; } @@ -119,7 +120,7 @@ protected SinkModuleBase(@Nonnull final Dependencies dependencies) { return null; } - final Evidence result = new Evidence(evidence, notMarkedRanges); + final Evidence result = buildEvidence(evidence, notMarkedRanges); report(span, type, result); return result; } @@ -161,7 +162,7 @@ protected SinkModuleBase(@Nonnull final Dependencies dependencies) { if (notMarkedRanges == null || notMarkedRanges.length == 0) { return null; } - final Evidence result = new Evidence(evidence.toString(), notMarkedRanges); + final Evidence result = buildEvidence(evidence, notMarkedRanges); report(span, type, result); return result; } @@ -179,6 +180,23 @@ protected StackTraceElement getCurrentStackTrace() { return stackWalker.walk(SinkModuleBase::findValidPackageForVulnerability); } + protected Evidence buildEvidence(final Object value, final Range[] ranges) { + if (Ranges.isUnbound(ranges)) { + final Range range = ranges[0]; + if (range != null && range.getSource() != null && range.getSource().getValue() != null) { + final String sourceValue = range.getSource().getValue(); + final String evidenceValue = value.toString(); + final int start = evidenceValue.indexOf(sourceValue); + if (start >= 0) { + return new Evidence( + evidenceValue, + new Range[] {new Range(start, sourceValue.length(), range.getSource(), NOT_MARKED)}); + } + } + } + return new Evidence(value instanceof String ? (String) value : value.toString(), ranges); + } + static StackTraceElement findValidPackageForVulnerability( @Nonnull final Stream stream) { final StackTraceElement[] first = new StackTraceElement[1]; diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java index 992655086c5..780996bb42a 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java @@ -59,6 +59,14 @@ public static Range[] forObject(final @Nonnull Source source, final int mark) { return new Range[] {new Range(0, Integer.MAX_VALUE, source, mark)}; } + public static boolean isUnbound(@Nonnull final Range[] ranges) { + if (ranges.length != 1) { + return false; + } + final Range range = ranges[0]; + return range.getStart() == 0 && range.getLength() == Integer.MAX_VALUE; + } + public static void copyShift( final @Nonnull Range[] src, final @Nonnull Range[] dst, final int dstPos, final int shift) { copyShift(src, dst, dstPos, shift, src.length); diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/AbstractSinkModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/AbstractSinkModuleTest.groovy index 1f2fa1f56a6..2474abf88e9 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/AbstractSinkModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/AbstractSinkModuleTest.groovy @@ -1,13 +1,37 @@ package com.datadog.iast.sink import com.datadog.iast.IastModuleImplTestBase +import com.datadog.iast.IastRequestContext +import com.datadog.iast.model.Source +import com.datadog.iast.overhead.Operations +import com.datadog.iast.propagation.PropagationModuleImpl +import com.datadog.iast.taint.Ranges +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.bootstrap.instrumentation.api.AgentSpan -class AbstractSinkModuleTest extends IastModuleImplTestBase { +import static com.datadog.iast.model.VulnerabilityType.SSRF +import static datadog.trace.api.iast.SourceTypes.REQUEST_PARAMETER_VALUE + +class AbstractSinkModuleTest extends IastModuleImplTestBase { final StackTraceElement ignoredPackageClassElement = element("org.springframework.Ignored") final StackTraceElement notIgnoredPackageClassElement = element("datadog.smoketest.NotIgnored") final StackTraceElement notInIastExclusionTrie = element("not.in.iast.exclusion.Class") + private IastRequestContext ctx + private AgentSpan span + + void setup() { + ctx = new IastRequestContext() + final reqCtx = Mock(RequestContext) { + getData(RequestContextSlot.IAST) >> ctx + } + span = Mock(AgentSpan) { + getRequestContext() >> reqCtx + } + tracer.activeSpan() >> span + } void 'filter ignored package element from stack'() { @@ -43,6 +67,37 @@ class AbstractSinkModuleTest extends IastModuleImplTestBase { result == expected } + void 'test reporting evidence on objects'() { + given: + overheadController.consumeQuota(Operations.REPORT_VULNERABILITY, span) >> true + final sink = new SinkModuleBase(dependencies) {} + final propagation = new PropagationModuleImpl() + final input = new String(source.value) + ctx.getTaintedObjects().taint(input, Ranges.forCharSequence(input, source)) + + when: + propagation.taintIfTainted(toReport, input) + final evidence = sink.checkInjection(span, ctx, SSRF, toReport) + + then: + evidence.ranges.length == 1 + final range = evidence.ranges[0] + if (matches) { + final taintedEvidence = evidence.value.substring(range.start, range.start + range.length) + taintedEvidence == input + } else { + final taintedEvidence = evidence.value + taintedEvidence != input + } + + where: + source | toReport | matches + new Source(REQUEST_PARAMETER_VALUE, 'url', 'datadog.com') | new URL('https://datadog.com/index.html') | true + new Source(REQUEST_PARAMETER_VALUE, 'url', 'datadog.com') | new URI('https://datadog.com/index.html') | true + new Source(REQUEST_PARAMETER_VALUE, 'url', 'datadog.com') | new URI('https://dAtAdOg.com/index.html') | false + new Source(REQUEST_PARAMETER_VALUE, 'url', 'datadog.com') | new URI('https://dAtAdOg.com/index.html') | false + } + private StackTraceElement element(final String declaringClass) { return new StackTraceElement(declaringClass, "method", "fileName", 1) }