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 a5a294456e1..8b7810f69c7 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 @@ -86,6 +86,9 @@ public interface VulnerabilityType { new VulnerabilityTypeImpl( VulnerabilityTypes.REFLECTION_INJECTION, VulnerabilityMarks.REFLECTION_INJECTION_MARK); + VulnerabilityType SESSION_REWRITING = + new ServiceVulnerabilityType(VulnerabilityTypes.SESSION_REWRITING); + String name(); /** A bit flag to ignore tainted ranges for this vulnerability. Set to 0 if none. */ @@ -192,4 +195,21 @@ public long calculateHash(@Nonnull final Vulnerability vulnerability) { return crc.getValue(); } } + + class ServiceVulnerabilityType extends VulnerabilityTypeImpl { + public ServiceVulnerabilityType(byte type, int... marks) { + super(type, marks); + } + + @Override + public long calculateHash(@Nonnull final Vulnerability vulnerability) { + CRC32 crc = new CRC32(); + update(crc, name()); + String serviceName = vulnerability.getLocation().getServiceName(); + if (serviceName != null) { + update(crc, serviceName); + } + return crc.getValue(); + } + } } 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 38ca75024ff..e653a758b9f 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 @@ -70,6 +70,8 @@ public class ApplicationModuleImpl extends SinkModuleBase implements Application public static final String WEB_XML = "web.xml"; + static final String SESSION_REWRITING_EVIDENCE_VALUE = "Servlet URL Session Tracking Mode"; + private static final Pattern PATTERN = Pattern.compile( Stream.of( @@ -103,6 +105,21 @@ public void onRealPath(final @Nullable String realPath) { checkWebXmlVulnerabilities(root, span); } + @Override + public void checkSessionTrackingModes(@Nonnull Set sessionTrackingModes) { + if (!sessionTrackingModes.contains("URL")) { + return; + } + final AgentSpan span = AgentTracer.activeSpan(); + // overhead is not checked here as it's called once per application context + reporter.report( + span, + new Vulnerability( + VulnerabilityType.SESSION_REWRITING, + Location.forSpan(span), + new Evidence(SESSION_REWRITING_EVIDENCE_VALUE))); + } + private void checkWebXmlVulnerabilities(@Nonnull Path path, AgentSpan span) { String webXmlContent = webXmlContent(path); if (webXmlContent == null) { diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/VulnerabilityTypeTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/VulnerabilityTypeTest.groovy index a73041bd45a..2d4f0d0dbd1 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/VulnerabilityTypeTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/VulnerabilityTypeTest.groovy @@ -6,6 +6,7 @@ import datadog.trace.test.util.DDSpecification import static com.datadog.iast.model.VulnerabilityType.INSECURE_COOKIE import static com.datadog.iast.model.VulnerabilityType.NO_HTTPONLY_COOKIE import static com.datadog.iast.model.VulnerabilityType.NO_SAMESITE_COOKIE +import static com.datadog.iast.model.VulnerabilityType.SESSION_REWRITING import static com.datadog.iast.model.VulnerabilityType.WEAK_CIPHER import static com.datadog.iast.model.VulnerabilityType.XCONTENTTYPE_HEADER_MISSING import static com.datadog.iast.model.VulnerabilityType.HSTS_HEADER_MISSING @@ -42,6 +43,9 @@ class VulnerabilityTypeTest extends DDSpecification { HSTS_HEADER_MISSING | getSpanLocation(123, null) | null | 121310697 HSTS_HEADER_MISSING | getSpanLocation(123, 'serviceName1') | null | 3533496951 HSTS_HEADER_MISSING | getSpanLocation(123, 'serviceName2') | null | 1268102093 + SESSION_REWRITING | getSpanLocation(123, null) | null | 2255304761 + SESSION_REWRITING | getSpanLocation(123, 'serviceName1') | null | 305779398 + SESSION_REWRITING | getSpanLocation(123, 'serviceName2') | null | 2335212412 } private Location getSpanAndStackLocation(final long spanId) { 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 ab249758beb..32b0947ce20 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 @@ -2,6 +2,8 @@ package com.datadog.iast.sink import com.datadog.iast.IastModuleImplTestBase import com.datadog.iast.Reporter +import com.datadog.iast.model.Vulnerability +import com.datadog.iast.model.VulnerabilityType import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.sink.ApplicationModule @@ -11,6 +13,7 @@ import static com.datadog.iast.model.VulnerabilityType.DIRECTORY_LISTING_LEAK import static com.datadog.iast.model.VulnerabilityType.INSECURE_JSP_LAYOUT import static com.datadog.iast.model.VulnerabilityType.SESSION_TIMEOUT import static com.datadog.iast.model.VulnerabilityType.VERB_TAMPERING +import static com.datadog.iast.sink.ApplicationModuleImpl.SESSION_REWRITING_EVIDENCE_VALUE class ApplicationModuleTest extends IastModuleImplTestBase { @@ -71,6 +74,31 @@ class ApplicationModuleTest extends IastModuleImplTestBase { 'application/defaulthtmlescapeinvalid/no_tag_2' | DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be set' | NO_LINE } + void 'iast module detects session rewriting on sessionTrackingModes'() { + when: + module.checkSessionTrackingModes(sessionTrackingModes as Set) + + then: + if (expected != null) { + 1 * reporter.report(_, _) >> { args -> assertSessionRewriting(args[1] as Vulnerability, expected) } + } else { + 0 * reporter.report(_, _) + } + + where: + sessionTrackingModes | expected + [] | null + ['COOKIE'] | null + ['URL'] | SESSION_REWRITING_EVIDENCE_VALUE + ['COOKIE', 'URL'] | SESSION_REWRITING_EVIDENCE_VALUE + } + + private static void assertSessionRewriting(final Vulnerability vuln, final String expected) { + assertVulnerability(vuln, VulnerabilityType.SESSION_REWRITING) + final evidence = vuln.getEvidence() + assert evidence.value == expected + } + private static void assertVulnerability(final vuln, final expectedVulnType) { assert vuln != null assert vuln.getType() == expectedVulnType diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/IastServlet3Advice.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/IastServlet3Advice.java index a62ee94a5c0..80de7211a60 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/IastServlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/IastServlet3Advice.java @@ -5,8 +5,11 @@ import datadog.trace.api.iast.VulnerabilityTypes; import datadog.trace.api.iast.sink.ApplicationModule; import datadog.trace.bootstrap.InstrumentationContext; +import java.util.HashSet; +import java.util.Set; import javax.servlet.ServletContext; import javax.servlet.ServletRequest; +import javax.servlet.SessionTrackingMode; import javax.servlet.http.HttpServletRequest; import net.bytebuddy.asm.Advice; @@ -27,6 +30,16 @@ public static void onExit(@Advice.Argument(0) ServletRequest request) { return; } InstrumentationContext.get(ServletContext.class, Boolean.class).put(context, true); - applicationModule.onRealPath(context.getRealPath("/")); + if (applicationModule != null) { + applicationModule.onRealPath(context.getRealPath("/")); + if (context.getEffectiveSessionTrackingModes() != null + && !context.getEffectiveSessionTrackingModes().isEmpty()) { + Set sessionTrackingModes = new HashSet<>(); + for (SessionTrackingMode mode : context.getEffectiveSessionTrackingModes()) { + sessionTrackingModes.add(mode.name()); + } + applicationModule.checkSessionTrackingModes(sessionTrackingModes); + } + } } } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/JettyServlet3Test.groovy index 608a54a3260..91976249092 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/JettyServlet3Test.groovy @@ -12,7 +12,6 @@ import org.eclipse.jetty.server.Request import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.servlet.ServletContextHandler - import javax.servlet.AsyncEvent import javax.servlet.AsyncListener import javax.servlet.Servlet @@ -52,7 +51,7 @@ abstract class JettyServlet3Test extends AbstractServlet3Test sessionTrackingModes = new HashSet<>(); + for (SessionTrackingMode mode : context.getEffectiveSessionTrackingModes()) { + sessionTrackingModes.add(mode.name()); + } + applicationModule.checkSessionTrackingModes(sessionTrackingModes); + } + } } } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/IastJakartaServletInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/IastJakartaServletInstrumentationTest.groovy index 3b574f5b223..4b7ab747631 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/IastJakartaServletInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/IastJakartaServletInstrumentationTest.groovy @@ -28,6 +28,7 @@ class IastJakartaServletInstrumentationTest extends AgentTestRunner{ then: 0 * appModule.onRealPath(_) + 0 * appModule.checkSessionTrackingModes(_) 0 * _ } @@ -44,6 +45,7 @@ class IastJakartaServletInstrumentationTest extends AgentTestRunner{ then: 1 * module.onRealPath(_) + 1 * module.checkSessionTrackingModes(['COOKIE', 'URL'] as Set) 0 * _ } } diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy index 78ab89de9c1..0369f4592fb 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy @@ -5,6 +5,7 @@ import okhttp3.MediaType import okhttp3.MultipartBody import okhttp3.Request import okhttp3.RequestBody +import okhttp3.Response import static datadog.trace.api.config.IastConfig.IAST_DEBUG_ENABLED import static datadog.trace.api.config.IastConfig.IAST_DETECTION_MODE @@ -968,6 +969,20 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { } } + void 'find session rewriting'() { + given: + String url = "http://localhost:${httpPort}/greeting" + + when: + Response response = client.newCall(new Request.Builder().url(url).get().build()).execute() + + then: + response.successful + hasVulnerabilityInLogs { vul -> + vul.type == 'SESSION_REWRITING' + } + } + } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java index b497dea01ea..ab09cd0b679 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java @@ -34,6 +34,7 @@ private VulnerabilityTypes() {} public static final byte HARDCODED_SECRET = 25; public static final byte INSECURE_AUTH_PROTOCOL = 26; public static final byte REFLECTION_INJECTION = 27; + public static final byte SESSION_REWRITING = 28; /** * Use for telemetry only, this is a special vulnerability type that is not reported, reported @@ -73,7 +74,8 @@ private VulnerabilityTypes() {} DEFAULT_HTML_ESCAPE_INVALID, SESSION_TIMEOUT, DIRECTORY_LISTING_LEAK, - INSECURE_JSP_LAYOUT + INSECURE_JSP_LAYOUT, + SESSION_REWRITING }; /** @@ -108,7 +110,8 @@ private VulnerabilityTypes() {} "ADMIN_CONSOLE_ACTIVE", "HARDCODED_SECRET", "INSECURE_AUTH_PROTOCOL", - "REFLECTION_INJECTION" + "REFLECTION_INJECTION", + "SESSION_REWRITING" }; public static String toString(final byte vulnerability) { diff --git a/internal-api/src/main/java/datadog/trace/api/iast/sink/ApplicationModule.java b/internal-api/src/main/java/datadog/trace/api/iast/sink/ApplicationModule.java index 795f2cac277..2282324cdc8 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/sink/ApplicationModule.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/sink/ApplicationModule.java @@ -2,10 +2,14 @@ import datadog.trace.api.iast.IastModule; import datadog.trace.api.iast.IastModule.OptOut; +import java.util.Set; +import javax.annotation.Nonnull; import javax.annotation.Nullable; @OptOut public interface ApplicationModule extends IastModule { void onRealPath(@Nullable String realPath); + + void checkSessionTrackingModes(@Nonnull Set sessionTrackingModes); } diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy index 0ffcada012c..f7d37f4ec7f 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy @@ -42,5 +42,6 @@ class VulnerabilityTypesTest extends DDSpecification { VulnerabilityTypes.HARDCODED_SECRET | 'HARDCODED_SECRET' VulnerabilityTypes.INSECURE_AUTH_PROTOCOL | 'INSECURE_AUTH_PROTOCOL' VulnerabilityTypes.REFLECTION_INJECTION | 'REFLECTION_INJECTION' + VulnerabilityTypes.SESSION_REWRITING | 'SESSION_REWRITING' } }