From bf9a508a0ff63ba790efe29745bfa79dcd0f67ea Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Sep 2024 08:35:23 +0200 Subject: [PATCH 01/20] SHI exploit prevention on one sink for java.lang.Runtime.exec(java.lang.String) --- .../config/AppSecConfigServiceImpl.java | 3 + .../appsec/event/data/KnownAddresses.java | 3 + .../datadog/appsec/gateway/GatewayBridge.java | 27 +++++++ ...ppSecConfigServiceImplSpecification.groovy | 4 + .../gateway/GatewayBridgeSpecification.groovy | 23 +++++- .../java/lang/RuntimeCallSite.java | 30 +++++--- .../java/lang/ShellCmdRaspHelper.java | 74 +++++++++++++++++++ .../lang/ShellCmdRaspHelperForkedTest.groovy | 73 ++++++++++++++++++ .../springboot/controller/WebController.java | 23 ++++++ .../appsec/SpringBootSmokeTest.groovy | 58 +++++++++++++++ .../datadog/trace/api/gateway/Events.java | 11 +++ .../api/gateway/InstrumentationGateway.java | 2 + .../datadog/trace/api/telemetry/RuleType.java | 3 +- .../gateway/InstrumentationGatewayTest.java | 4 + 14 files changed, 327 insertions(+), 11 deletions(-) create mode 100644 dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java create mode 100644 dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java index c2cb5fc7edc..652cb398851 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java @@ -13,6 +13,7 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI; +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING; @@ -118,6 +119,7 @@ private void subscribeConfigurationPoller() { capabilities |= CAPABILITY_ASM_RASP_SQLI; capabilities |= CAPABILITY_ASM_RASP_SSRF; capabilities |= CAPABILITY_ASM_RASP_LFI; + capabilities |= CAPABILITY_ASM_RASP_SHI; } this.configurationPoller.addCapabilities(capabilities); } @@ -362,6 +364,7 @@ public void close() { | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_LFI + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index 65649e149ec..7e63227b902 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -133,6 +133,9 @@ public interface KnownAddresses { Address> WAF_CONTEXT_PROCESSOR = new Address<>("waf.context.processor"); + /** The Shell command being executed */ + Address SHELL_CMD = new Address<>("server.sys.shell.cmd"); + static Address forName(String name) { switch (name) { case "server.request.body": diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index e27848c39f0..8e9b2718a67 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -94,6 +94,7 @@ public class GatewayBridge { private volatile DataSubscriberInfo sessionIdSubInfo; private final ConcurrentHashMap, DataSubscriberInfo> userIdSubInfo = new ConcurrentHashMap<>(); + private volatile DataSubscriberInfo shellCmdSubInfo; public GatewayBridge( SubscriptionService subscriptionService, @@ -140,6 +141,7 @@ public void init() { EVENTS.loginSuccess(), this.onUserEvent(KnownAddresses.LOGIN_SUCCESS)); subscriptionService.registerCallback( EVENTS.loginFailure(), this.onUserEvent(KnownAddresses.LOGIN_FAILURE)); + subscriptionService.registerCallback(EVENTS.shellCmd(), this::onShellCmd); if (additionalIGEvents.contains(EVENTS.requestPathParams())) { subscriptionService.registerCallback(EVENTS.requestPathParams(), this::onRequestPathParams); @@ -258,6 +260,31 @@ private Flow onNetworkConnection(RequestContext ctx_, String url) { } } + private Flow onShellCmd(RequestContext ctx_, String command) { + AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); + if (ctx == null) { + return NoopFlow.INSTANCE; + } + while (true) { + DataSubscriberInfo subInfo = shellCmdSubInfo; + if (subInfo == null) { + subInfo = producerService.getDataSubscribers(KnownAddresses.SHELL_CMD); + shellCmdSubInfo = subInfo; + } + if (subInfo == null || subInfo.isEmpty()) { + return NoopFlow.INSTANCE; + } + DataBundle bundle = + new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.SHELL_CMD, command).build(); + try { + GatewayContext gwCtx = new GatewayContext(true, RuleType.SHI); + return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); + } catch (ExpiredSubscriberInfoException e) { + shellCmdSubInfo = null; + } + } + } + private Flow onFileLoaded(RequestContext ctx_, String path) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy index c07c430f5de..670714cab04 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy @@ -27,6 +27,7 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_HEADER_FINGERPRIN import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING @@ -271,6 +272,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_TRUSTED_IPS | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -423,6 +425,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_TRUSTED_IPS | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -496,6 +499,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_API_SECURITY_SAMPLE_RATE | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 2cc6cbc5170..50045873dd1 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -92,7 +92,7 @@ class GatewayBridgeSpecification extends DDSpecification { TriFunction> userIdCB TriFunction> loginSuccessCB TriFunction> loginFailureCB - + BiFunction> shellCmdCB void setup() { callInitAndCaptureCBs() @@ -432,6 +432,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * ig.registerCallback(EVENTS.userId(), _) >> { userIdCB = it[1]; null } 1 * ig.registerCallback(EVENTS.loginSuccess(), _) >> { loginSuccessCB = it[1]; null } 1 * ig.registerCallback(EVENTS.loginFailure(), _) >> { loginFailureCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.shellCmd(), _) >> { shellCmdCB = it[1]; null } 0 * ig.registerCallback(_, _) bridge.init() @@ -834,6 +835,26 @@ class GatewayBridgeSpecification extends DDSpecification { gatewayContext.isRasp == true } + void 'process shell cmd'() { + setup: + final cmd = '<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->' + eventDispatcher.getDataSubscribers({ KnownAddresses.SHELL_CMD in it }) >> nonEmptyDsInfo + DataBundle bundle + GatewayContext gatewayContext + + when: + Flow flow = shellCmdCB.apply(ctx, cmd) + + then: + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> + { a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE } + bundle.get(KnownAddresses.SHELL_CMD) == cmd + flow.result == null + flow.action == Flow.Action.Noop.INSTANCE + gatewayContext.isTransient == true + gatewayContext.isRasp == true + } + void 'calls trace segment post processor'() { setup: AgentSpan span = Stub() diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java index f6ae37be422..5f7149d2aef 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.java.lang; import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.appsec.RaspCallSites; import datadog.trace.api.iast.IastCallSites; import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Sink; @@ -10,20 +11,16 @@ import javax.annotation.Nullable; @Sink(VulnerabilityTypes.COMMAND_INJECTION) -@CallSite(spi = IastCallSites.class) +@CallSite( + spi = {IastCallSites.class, RaspCallSites.class}, + helpers = ShellCmdRaspHelper.class) public class RuntimeCallSite { @CallSite.Before("java.lang.Process java.lang.Runtime.exec(java.lang.String)") public static void beforeStart(@CallSite.Argument @Nullable final String command) { if (command != null) { // runtime fails if null - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(command); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(command); + raspCallback(command); } } @@ -109,4 +106,19 @@ public static void beforeExec( } } } + + private static void iastCallback(String command) { + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(command); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } + } + + private static void raspCallback(String command) { + ShellCmdRaspHelper.INSTANCE.beforeShellCmd(command); + } } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java new file mode 100644 index 00000000000..7099b9741aa --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java @@ -0,0 +1,74 @@ +package datadog.trace.instrumentation.java.lang; + +import static datadog.trace.api.gateway.Events.EVENTS; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import java.util.function.BiFunction; +import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ShellCmdRaspHelper { + + public static ShellCmdRaspHelper INSTANCE = new ShellCmdRaspHelper(); + + private static final Logger LOGGER = LoggerFactory.getLogger(ShellCmdRaspHelper.class); + + private ShellCmdRaspHelper() { + // prevent instantiation + } + + public void beforeShellCmd(@Nonnull final String cmd) { + if (!Config.get().isAppSecRaspEnabled()) { + return; + } + try { + final BiFunction> shellCmdCallback = + AgentTracer.get() + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.shellCmd()); + + if (shellCmdCallback == null) { + return; + } + + final AgentSpan span = AgentTracer.get().activeSpan(); + if (span == null) { + return; + } + + final RequestContext ctx = span.getRequestContext(); + if (ctx == null) { + return; + } + + Flow flow = shellCmdCallback.apply(ctx, cmd); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + brf.tryCommitBlockingResponse( + ctx.getTraceSegment(), + rba.getStatusCode(), + rba.getBlockingContentType(), + rba.getExtraHeaders()); + } + throw new BlockingException("Blocked request (for SHI attempt)"); + } + } catch (final BlockingException e) { + // re-throw blocking exceptions + throw e; + } catch (final Throwable e) { + // suppress anything else + LOGGER.debug("Exception during SHI rasp callback", e); + } + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy new file mode 100644 index 00000000000..8a4682e67b9 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy @@ -0,0 +1,73 @@ +package datadog.trace.instrumentation.java.lang + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.config.AppSecConfig +import datadog.trace.api.config.IastConfig +import datadog.trace.api.gateway.CallbackProvider +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.internal.TraceSegment +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import spock.lang.Shared + +import java.util.function.BiFunction + +import static datadog.trace.api.gateway.Events.EVENTS + +class ShellCmdRaspHelperForkedTest extends AgentTestRunner { + + @Shared + protected static final ORIGINAL_TRACER = AgentTracer.get() + + protected traceSegment + protected reqCtx + protected span + protected tracer + + void setup() { + traceSegment = Stub(TraceSegment) + reqCtx = Stub(RequestContext) { + getTraceSegment() >> traceSegment + } + span = Stub(AgentSpan) { + getRequestContext() >> reqCtx + } + tracer = Stub(AgentTracer.TracerAPI) { + activeSpan() >> span + } + AgentTracer.forceRegister(tracer) + } + + void cleanup() { + AgentTracer.forceRegister(ORIGINAL_TRACER) + } + + @Override + protected void configurePreAgent() { + injectSysConfig(IastConfig.IAST_ENABLED, 'true') + injectSysConfig(AppSecConfig.APPSEC_ENABLED, 'true') + injectSysConfig(AppSecConfig.APPSEC_RASP_ENABLED, 'true') + } + + void 'test Helper'() { + + setup: + final callbackProvider = Mock(CallbackProvider) + final listener = Mock(BiFunction) + final flow = Mock(Flow) + tracer.getCallbackProvider(RequestContextSlot.APPSEC) >> callbackProvider + + when: + ShellCmdRaspHelper.INSTANCE.beforeShellCmd(*args) + + then: + 1 * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener + 1 * listener.apply(reqCtx, expected) >> flow + + where: + args | expected + ['<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->'] | '<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->' + } +} diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 6f69f9dd29f..7c802420271 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -158,4 +158,27 @@ public ResponseEntity session(final HttpServletRequest request) { final HttpSession session = request.getSession(true); return new ResponseEntity<>(session.getId(), HttpStatus.OK); } + + @GetMapping("/shi/cmd") + public String shiCmd(@RequestParam("cmd") String cmd) { + withProcess(() -> Runtime.getRuntime().exec(cmd)); + return "EXECUTED"; + } + + private void withProcess(final Operation op) { + Process process = null; + try { + process = op.run(); + } catch (final Throwable e) { + // ignore it + } finally { + if (process != null && process.isAlive()) { + process.destroyForcibly(); + } + } + } + + private interface Operation { + E run() throws Throwable; + } } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index adb0d15a85c..7b0e5dc3dc0 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -150,6 +150,30 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { ], transformers: [], on_match : ['block'] + ], + [ + id : '__test_shi_block', + name : 'Shell injection exploit', + enable : 'true', + tags : [ + type: 'command_injection', + category: 'vulnerability_trigger', + cwe: '77', + capec: '1000/152/248/88', + confidence: '0', + module: 'rasp' + ], + conditions : [ + [ + parameters: [ + resource: [[address: 'server.sys.shell.cmd']], + params : [[address: 'server.request.query']], + ], + operator : "shi_detector", + ], + ], + transformers: [], + on_match : ['block'] ] ]) } @@ -503,4 +527,38 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } assert trigger != null, 'test trigger not found' } + + void 'rasp blocks on SHI'() { + when: + String url = "http://localhost:${httpPort}/shi/cmd?cmd=%0Aid%0A" + def request = new Request.Builder() + .url(url) + .get() + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + response.code() == 403 + responseBodyStr.contains('You\'ve been blocked') + + when: + waitForTraceCount(1) + + then: + def rootSpans = this.rootSpans.toList() + rootSpans.size() == 1 + def rootSpan = rootSpans[0] + assert rootSpan.meta.get('appsec.blocked') == 'true', 'appsec.blocked is not set' + assert rootSpan.meta.get('_dd.appsec.json') != null, '_dd.appsec.json is not set' + def trigger = null + for (t in rootSpan.triggers) { + if (t['rule']['id'] == '__test_shi_block') { + trigger = t + break + } + } + assert trigger != null, 'test trigger not found' + } + } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index df111af28ee..a1851476ae9 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -308,6 +308,17 @@ public EventType("shell.cmd", SHELL_CDM_ID); + + /** A I/O network URL */ + @SuppressWarnings("unchecked") + public EventType>> shellCmd() { + return (EventType>>) SHELL_CMD; + } + static final int MAX_EVENTS = nextId.get(); private static final class ET extends EventType { diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 45d734f0d11..cb0c33e9b96 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -26,6 +26,7 @@ import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_ID; import static datadog.trace.api.gateway.Events.RESPONSE_STARTED_ID; import static datadog.trace.api.gateway.Events.USER_ID; +import static datadog.trace.api.gateway.Events.SHELL_CDM_ID; import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.function.TriConsumer; @@ -418,6 +419,7 @@ public Flow apply(RequestContext ctx, String arg) { case DATABASE_SQL_QUERY_ID: case NETWORK_CONNECTION_ID: case FILE_LOADED_ID: + case SHELL_CDM_ID: return (C) new BiFunction>() { @Override diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java index 64705e7b0f0..bae7501122b 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java @@ -3,7 +3,8 @@ public enum RuleType { LFI("lfi"), SQL_INJECTION("sql_injection"), - SSRF("ssrf"); + SSRF("ssrf"), + SHI("shi"); public final String name; private static final int numValues = RuleType.values().length; diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index ea441fd372a..1ee0c2f78ab 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -221,6 +221,8 @@ public void testNormalCalls() { cbp.getCallback(events.loginSuccess()).apply(null, null, null); ss.registerCallback(events.loginFailure(), callback); cbp.getCallback(events.loginFailure()).apply(null, null, null); + ss.registerCallback(events.shellCmd(), callback); + cbp.getCallback(events.shellCmd()).apply(null, null); assertThat(callback.count).isEqualTo(Events.MAX_EVENTS); } @@ -289,6 +291,8 @@ public void testThrowableBlocking() { cbp.getCallback(events.loginSuccess()).apply(null, null, null); ss.registerCallback(events.loginFailure(), throwback); cbp.getCallback(events.loginFailure()).apply(null, null, null); + ss.registerCallback(events.shellCmd(), callback); + cbp.getCallback(events.shellCmd()).apply(null, null); assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS); } From bf040139a45517e037f89c1047f980c1fff7dcdc Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 28 Nov 2024 11:49:44 +0100 Subject: [PATCH 02/20] fix spotless --- .../java/datadog/trace/api/gateway/InstrumentationGateway.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index cb0c33e9b96..137ba9f8848 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -25,8 +25,8 @@ import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_DONE_ID; import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_ID; import static datadog.trace.api.gateway.Events.RESPONSE_STARTED_ID; -import static datadog.trace.api.gateway.Events.USER_ID; import static datadog.trace.api.gateway.Events.SHELL_CDM_ID; +import static datadog.trace.api.gateway.Events.USER_ID; import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.function.TriConsumer; From 7043b2ca00e837bbbc3e5623699772fb3533b3e5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 29 Nov 2024 09:07:12 +0100 Subject: [PATCH 03/20] first steps to add cmdArray support (not blocking) --- .../java/lang/RuntimeCallSite.java | 89 ++++++++++--------- .../java/lang/ShellCmdRaspHelper.java | 4 + 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java index 5f7149d2aef..102797d7fa9 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java @@ -27,14 +27,8 @@ public static void beforeStart(@CallSite.Argument @Nullable final String command @CallSite.Before("java.lang.Process java.lang.Runtime.exec(java.lang.String[])") public static void beforeExec(@CallSite.Argument @Nullable final String[] cmdArray) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(cmdArray); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(cmdArray); + raspCallback(cmdArray); } } @@ -43,14 +37,8 @@ public static void beforeExec( @CallSite.Argument @Nullable final String command, @CallSite.Argument @Nullable final String[] envp) { if (command != null) { // runtime fails if null - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, command); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(envp, command); + raspCallback(command); } } @@ -60,14 +48,8 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] cmdArray, @CallSite.Argument @Nullable final String[] envp) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, cmdArray); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(envp, cmdArray); + raspCallback(cmdArray); } } @@ -78,14 +60,8 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] envp, @CallSite.Argument @Nullable final File dir) { if (command != null) { // runtime fails if null - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, command); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(envp, command); + raspCallback(command); } } @@ -96,14 +72,8 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] envp, @CallSite.Argument @Nullable final File dir) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, cmdArray); - } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); - } - } + iastCallback(envp, cmdArray); + raspCallback(cmdArray); } } @@ -113,7 +83,40 @@ private static void iastCallback(String command) { try { module.onRuntimeExec(command); } catch (final Throwable e) { - module.onUnexpectedException("beforeExec threw", e); + module.onUnexpectedException("iastCallback threw", e); + } + } + } + + private static void iastCallback(String[] cmdArray) { + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(cmdArray); + } catch (final Throwable e) { + module.onUnexpectedException("iastCallback threw", e); + } + } + } + + private static void iastCallback(String[] envp, String command) { + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, command); + } catch (final Throwable e) { + module.onUnexpectedException("iastCallback threw", e); + } + } + } + + private static void iastCallback(String[] envp, String[] cmdArray) { + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, cmdArray); + } catch (final Throwable e) { + module.onUnexpectedException("iastCallback threw", e); } } } @@ -121,4 +124,8 @@ private static void iastCallback(String command) { private static void raspCallback(String command) { ShellCmdRaspHelper.INSTANCE.beforeShellCmd(command); } + + private static void raspCallback(String[] cmdArray) { + ShellCmdRaspHelper.INSTANCE.beforeShellCmd(cmdArray); + } } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java index 7099b9741aa..ba0ee0cf78c 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java @@ -25,6 +25,10 @@ private ShellCmdRaspHelper() { // prevent instantiation } + public void beforeShellCmd(@Nonnull final String[] cmdArray) { + // TODO + } + public void beforeShellCmd(@Nonnull final String cmd) { if (!Config.get().isAppSecRaspEnabled()) { return; From f3bb30fffb44fca6f478b18db4b5fc60006727b2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 13:42:07 +0100 Subject: [PATCH 04/20] Fix known addresses --- .../com/datadog/appsec/event/data/KnownAddresses.java | 2 ++ .../appsec/springboot/controller/WebController.java | 2 +- .../datadog/smoketest/appsec/SpringBootSmokeTest.groovy | 9 ++++++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index 7e63227b902..2dfba57a6c5 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -208,6 +208,8 @@ static Address forName(String name) { return LOGIN_SUCCESS; case "server.business_logic.users.login.failure": return LOGIN_FAILURE; + case "server.sys.shell.cmd": + return SHELL_CMD; default: return null; } diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 7c802420271..504aa627142 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -159,7 +159,7 @@ public ResponseEntity session(final HttpServletRequest request) { return new ResponseEntity<>(session.getId(), HttpStatus.OK); } - @GetMapping("/shi/cmd") + @PostMapping("/shi/cmd") public String shiCmd(@RequestParam("cmd") String cmd) { withProcess(() -> Runtime.getRuntime().exec(cmd)); return "EXECUTED"; diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 7b0e5dc3dc0..5eb5b72f8e3 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -2,6 +2,7 @@ package datadog.smoketest.appsec import datadog.trace.agent.test.utils.OkHttpUtils import datadog.trace.agent.test.utils.ThreadUtils +import okhttp3.FormBody import okhttp3.MediaType import okhttp3.Request import okhttp3.RequestBody @@ -167,7 +168,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { [ parameters: [ resource: [[address: 'server.sys.shell.cmd']], - params : [[address: 'server.request.query']], + params : [[address: 'server.request.body']], ], operator : "shi_detector", ], @@ -188,6 +189,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { List command = new ArrayList<>() command.add(javaPath()) + command.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005") command.addAll(defaultJavaProperties) command.addAll(defaultAppSecProperties) command.addAll((String[]) ["-jar", springBootShadowJar, "--server.port=${httpPort}"]) @@ -530,10 +532,11 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { void 'rasp blocks on SHI'() { when: - String url = "http://localhost:${httpPort}/shi/cmd?cmd=%0Aid%0A" + String url = "http://localhost:${httpPort}/shi/cmd" + final body = new FormBody.Builder().add("cmd", "cat etc/password").build() def request = new Request.Builder() .url(url) - .get() + .post(body) .build() def response = client.newCall(request).execute() def responseBodyStr = response.body().string() From 8323532e8d8c9745f168838e81b662e6f4c22c0c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 14:02:07 +0100 Subject: [PATCH 05/20] Fix test --- .../groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 5eb5b72f8e3..d2549be7139 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -189,7 +189,6 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { List command = new ArrayList<>() command.add(javaPath()) - command.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005") command.addAll(defaultJavaProperties) command.addAll(defaultAppSecProperties) command.addAll((String[]) ["-jar", springBootShadowJar, "--server.port=${httpPort}"]) From 663712b38eb14d3dc0a17562c6f58012cc77f982 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 15:42:36 +0100 Subject: [PATCH 06/20] Add support for arrayCmd methods and more smoke tests --- .../datadog/appsec/gateway/GatewayBridge.java | 2 +- .../java/lang/ShellCmdRaspHelper.java | 8 +++-- .../lang/ShellCmdRaspHelperForkedTest.groovy | 6 ++-- .../springboot/controller/WebController.java | 34 +++++++++++++++++++ .../appsec/SpringBootSmokeTest.groovy | 22 ++++++++++-- .../datadog/trace/api/gateway/Events.java | 5 ++- .../api/gateway/InstrumentationGateway.java | 15 +++++++- 7 files changed, 80 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 8e9b2718a67..cc5e28058eb 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -260,7 +260,7 @@ private Flow onNetworkConnection(RequestContext ctx_, String url) { } } - private Flow onShellCmd(RequestContext ctx_, String command) { + private Flow onShellCmd(RequestContext ctx_, Object command) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { return NoopFlow.INSTANCE; diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java index ba0ee0cf78c..9ed26af5f4d 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java @@ -26,15 +26,19 @@ private ShellCmdRaspHelper() { } public void beforeShellCmd(@Nonnull final String[] cmdArray) { - // TODO + shellCmd(cmdArray); } public void beforeShellCmd(@Nonnull final String cmd) { + shellCmd(cmd); + } + + private void shellCmd(Object cmd) { if (!Config.get().isAppSecRaspEnabled()) { return; } try { - final BiFunction> shellCmdCallback = + final BiFunction> shellCmdCallback = AgentTracer.get() .getCallbackProvider(RequestContextSlot.APPSEC) .getCallback(EVENTS.shellCmd()); diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy index 8a4682e67b9..7299197d56e 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy @@ -46,7 +46,6 @@ class ShellCmdRaspHelperForkedTest extends AgentTestRunner { @Override protected void configurePreAgent() { - injectSysConfig(IastConfig.IAST_ENABLED, 'true') injectSysConfig(AppSecConfig.APPSEC_ENABLED, 'true') injectSysConfig(AppSecConfig.APPSEC_RASP_ENABLED, 'true') } @@ -67,7 +66,8 @@ class ShellCmdRaspHelperForkedTest extends AgentTestRunner { 1 * listener.apply(reqCtx, expected) >> flow where: - args | expected - ['<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->'] | '<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->' + args | expected + ['cat etc/password'] | 'cat etc/password' + [['cat etc/password', 'cat etc/password'] as String[]] | ['cat etc/password', 'cat etc/password'] } } diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 504aa627142..36462350b5a 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -159,12 +159,46 @@ public ResponseEntity session(final HttpServletRequest request) { return new ResponseEntity<>(session.getId(), HttpStatus.OK); } + @PostMapping("/shi/arrayCmd") + public String shiArrayCmd(@RequestParam("cmd") String[] arrayCmd) { + withProcess(() -> Runtime.getRuntime().exec(arrayCmd)); + return "EXECUTED"; + } + @PostMapping("/shi/cmd") public String shiCmd(@RequestParam("cmd") String cmd) { withProcess(() -> Runtime.getRuntime().exec(cmd)); return "EXECUTED"; } + @PostMapping("/shi/arrayCmdWithParams") + public String shiArrayCmdWithParams( + @RequestParam("cmd") String[] arrayCmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(arrayCmd, params)); + return "EXECUTED"; + } + + @PostMapping("/shi/cmdWithParams") + public String shiCmdWithParams( + @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(cmd, params)); + return "EXECUTED"; + } + + @PostMapping("/shi/arrayCmdWithParamsAndFile") + public String shiArrayCmdWithParamsAndFile( + @RequestParam("cmd") String[] arrayCmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(arrayCmd, params, new File(""))); + return "EXECUTED"; + } + + @PostMapping("/shi/cmdParamsAndFile") + public String shiCmdParamsAndFile( + @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(cmd, params, new File(""))); + return "EXECUTED"; + } + private void withProcess(final Operation op) { Process process = null; try { diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index d2549be7139..c322b539516 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -531,8 +531,17 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { void 'rasp blocks on SHI'() { when: - String url = "http://localhost:${httpPort}/shi/cmd" - final body = new FormBody.Builder().add("cmd", "cat etc/password").build() + String url = "http://localhost:${httpPort}/shi/"+endpoint + def formBuilder = new FormBody.Builder() + for (s in cmd) { + formBuilder.add("cmd", s) + } + if (params != null) { + for (s in params) { + formBuilder.add("params", s) + } + } + final body = formBuilder.build() def request = new Request.Builder() .url(url) .post(body) @@ -561,6 +570,15 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { } } assert trigger != null, 'test trigger not found' + + where: + endpoint | cmd | params + 'cmd' | ['cat etc/password'] | null + 'arrayCmd' | ['cat etc/password', 'cat etc/password'] | null + 'cmdWithParams' | ['cat etc/password'] | ['param'] + 'arrayCmdWithParams' | ['cat etc/password', 'cat etc/password'] | ['param'] + 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] + 'arrayCmdWithParamsAndFile' | ['cat etc/password', 'cat etc/password'] | ['param'] } } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index a1851476ae9..83c23cb18c3 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -313,10 +313,9 @@ public EventType("shell.cmd", SHELL_CDM_ID); - /** A I/O network URL */ @SuppressWarnings("unchecked") - public EventType>> shellCmd() { - return (EventType>>) SHELL_CMD; + public EventType>> shellCmd() { + return (EventType>>) SHELL_CMD; } static final int MAX_EVENTS = nextId.get(); diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 137ba9f8848..816e74b3200 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -419,7 +419,6 @@ public Flow apply(RequestContext ctx, String arg) { case DATABASE_SQL_QUERY_ID: case NETWORK_CONNECTION_ID: case FILE_LOADED_ID: - case SHELL_CDM_ID: return (C) new BiFunction>() { @Override @@ -433,6 +432,20 @@ public Flow apply(RequestContext ctx, String arg) { } } }; + case SHELL_CDM_ID: + return (C) + new BiFunction>() { + @Override + public Flow apply(RequestContext ctx, Object arg) { + try { + return ((BiFunction>) callback) + .apply(ctx, arg); + } catch (Throwable t) { + log.warn("Callback for {} threw.", eventType, t); + return Flow.ResultFlow.empty(); + } + } + }; default: log.warn("Unwrapped callback for {}", eventType); return callback; From 68f08b245c440d3b956f74cf61d789482b2e7dcd Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 15:42:36 +0100 Subject: [PATCH 07/20] Add support for arrayCmd methods and more smoke tests --- .../datadog/appsec/gateway/GatewayBridgeSpecification.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 50045873dd1..87e549e7015 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -92,7 +92,7 @@ class GatewayBridgeSpecification extends DDSpecification { TriFunction> userIdCB TriFunction> loginSuccessCB TriFunction> loginFailureCB - BiFunction> shellCmdCB + BiFunction> shellCmdCB void setup() { callInitAndCaptureCBs() From 3b62d1c83b126d59d8a1d4d63aa3a15ecf85b5f1 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 15:42:36 +0100 Subject: [PATCH 08/20] Add support for arrayCmd methods and more smoke tests --- .../java/com/datadog/appsec/event/data/KnownAddresses.java | 4 ++-- .../appsec/event/data/KnownAddressesSpecification.groovy | 7 +++++-- .../java/lang/ShellCmdRaspHelperForkedTest.groovy | 1 - .../trace/api/gateway/InstrumentationGatewayTest.java | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index 2dfba57a6c5..a0b2eeba6f0 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -131,11 +131,11 @@ public interface KnownAddresses { /** Login success business event */ Address LOGIN_SUCCESS = new Address<>("server.business_logic.users.login.success"); - Address> WAF_CONTEXT_PROCESSOR = new Address<>("waf.context.processor"); - /** The Shell command being executed */ Address SHELL_CMD = new Address<>("server.sys.shell.cmd"); + Address> WAF_CONTEXT_PROCESSOR = new Address<>("waf.context.processor"); + static Address forName(String name) { switch (name) { case "server.request.body": diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy index 1d285fc7913..a3e886efce4 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy @@ -39,13 +39,16 @@ class KnownAddressesSpecification extends Specification { 'usr.session_id', 'server.business_logic.users.login.failure', 'server.business_logic.users.login.success', - 'waf.context.processor', + 'server.io.net.url', + 'server.io.fs.file', + 'server.sys.shell.cmd', + 'waf.context.processor' ] } void 'number of known addresses is expected number'() { expect: - Address.instanceCount() == 35 + Address.instanceCount() == 36 KnownAddresses.WAF_CONTEXT_PROCESSOR.serial == Address.instanceCount() - 1 } } diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy index 7299197d56e..27b49c995d4 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy @@ -2,7 +2,6 @@ package datadog.trace.instrumentation.java.lang import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.config.AppSecConfig -import datadog.trace.api.config.IastConfig import datadog.trace.api.gateway.CallbackProvider import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.RequestContext diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index 1ee0c2f78ab..85527f55181 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -291,7 +291,7 @@ public void testThrowableBlocking() { cbp.getCallback(events.loginSuccess()).apply(null, null, null); ss.registerCallback(events.loginFailure(), throwback); cbp.getCallback(events.loginFailure()).apply(null, null, null); - ss.registerCallback(events.shellCmd(), callback); + ss.registerCallback(events.shellCmd(), throwback); cbp.getCallback(events.shellCmd()).apply(null, null); assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS); } From d7226e1747513d9f7d3ce038fa74059b35b505d5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 10 Dec 2024 17:18:24 +0100 Subject: [PATCH 09/20] Add support for ProcessBuilder --- .../java/lang/ProcessBuilderCallSite.java | 28 +++++++++++++------ .../springboot/controller/WebController.java | 6 ++++ .../appsec/SpringBootSmokeTest.groovy | 13 +++++---- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java index 0d88401bdb8..85e33715ba1 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.java.lang; import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.appsec.RaspCallSites; import datadog.trace.api.iast.IastCallSites; import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Sink; @@ -11,7 +12,9 @@ // TODO deal with the environment @Sink(VulnerabilityTypes.COMMAND_INJECTION) -@CallSite(spi = IastCallSites.class) +@CallSite( + spi = {IastCallSites.class, RaspCallSites.class}, + helpers = ShellCmdRaspHelper.class) public class ProcessBuilderCallSite { @CallSite.Before("java.lang.Process java.lang.ProcessBuilder.start()") @@ -22,14 +25,23 @@ public static void beforeStart(@CallSite.This @Nullable final ProcessBuilder sel // be careful when fetching the environment as it does mutate the instance final List cmd = self.command(); if (cmd != null && cmd.size() > 0) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onProcessBuilderStart(cmd); - } catch (final Throwable e) { - module.onUnexpectedException("beforeStart threw", e); - } + iastCallback(cmd); + raspCallback(cmd); + } + } + + private static void iastCallback(List cmd) { + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onProcessBuilderStart(cmd); + } catch (final Throwable e) { + module.onUnexpectedException("beforeStart threw", e); } } } + + private static void raspCallback(List list) { + ShellCmdRaspHelper.INSTANCE.beforeShellCmd(list.toArray(new String[0])); + } } diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 36462350b5a..1a590fca21d 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -199,6 +199,12 @@ public String shiCmdParamsAndFile( return "EXECUTED"; } + @PostMapping("/shi/processBuilder") + public String shiProcessBuilder(@RequestParam("cmd") String[] cmd) { + withProcess(() -> new ProcessBuilder(cmd).start()); + return "EXECUTED"; + } + private void withProcess(final Operation op) { Process process = null; try { diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index c322b539516..aee0fb550d8 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -572,13 +572,14 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { assert trigger != null, 'test trigger not found' where: - endpoint | cmd | params - 'cmd' | ['cat etc/password'] | null - 'arrayCmd' | ['cat etc/password', 'cat etc/password'] | null - 'cmdWithParams' | ['cat etc/password'] | ['param'] - 'arrayCmdWithParams' | ['cat etc/password', 'cat etc/password'] | ['param'] - 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] + endpoint | cmd | params + 'cmd' | ['cat etc/password'] | null + 'arrayCmd' | ['cat etc/password', 'cat etc/password'] | null + 'cmdWithParams' | ['cat etc/password'] | ['param'] + 'arrayCmdWithParams' | ['cat etc/password', 'cat etc/password'] | ['param'] + 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] 'arrayCmdWithParamsAndFile' | ['cat etc/password', 'cat etc/password'] | ['param'] + 'processBuilder' | ['cat etc/password', 'cat etc/password'] | null } } From b488d8a3c7689755d2c3c7e61bc440a37264857a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 11 Dec 2024 13:40:38 +0100 Subject: [PATCH 10/20] Move to ProcessImplInstrumentation approach --- .../datadog/appsec/gateway/GatewayBridge.java | 2 +- .../gateway/GatewayBridgeSpecification.groovy | 2 +- .../java/lang/ProcessBuilderCallSite.java | 28 ++--- .../java/lang/ProcessImplStartAdvice.java | 1 + .../java/lang/RuntimeCallSite.java | 115 ++++++++---------- .../java/lang/ShellCmdRaspHelper.java | 82 ------------- ...trumentationShellCmdRaspForkedTest.groovy} | 14 +-- .../appsec/SpringBootSmokeTest.groovy | 7 +- .../datadog/trace/api/gateway/Events.java | 4 +- .../api/gateway/InstrumentationGateway.java | 6 +- .../ProcessImplInstrumentationHelpers.java | 65 ++++++++++ 11 files changed, 138 insertions(+), 188 deletions(-) delete mode 100644 dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java rename dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/{ShellCmdRaspHelperForkedTest.groovy => ProcessImplInstrumentationShellCmdRaspForkedTest.groovy} (81%) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index cc5e28058eb..d2ebe72dd9f 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -260,7 +260,7 @@ private Flow onNetworkConnection(RequestContext ctx_, String url) { } } - private Flow onShellCmd(RequestContext ctx_, Object command) { + private Flow onShellCmd(RequestContext ctx_, String[] command) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { return NoopFlow.INSTANCE; diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 87e549e7015..f1503f5d6a8 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -92,7 +92,7 @@ class GatewayBridgeSpecification extends DDSpecification { TriFunction> userIdCB TriFunction> loginSuccessCB TriFunction> loginFailureCB - BiFunction> shellCmdCB + BiFunction> shellCmdCB void setup() { callInitAndCaptureCBs() diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java index 85e33715ba1..0d88401bdb8 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSite.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.java.lang; import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.appsec.RaspCallSites; import datadog.trace.api.iast.IastCallSites; import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Sink; @@ -12,9 +11,7 @@ // TODO deal with the environment @Sink(VulnerabilityTypes.COMMAND_INJECTION) -@CallSite( - spi = {IastCallSites.class, RaspCallSites.class}, - helpers = ShellCmdRaspHelper.class) +@CallSite(spi = IastCallSites.class) public class ProcessBuilderCallSite { @CallSite.Before("java.lang.Process java.lang.ProcessBuilder.start()") @@ -25,23 +22,14 @@ public static void beforeStart(@CallSite.This @Nullable final ProcessBuilder sel // be careful when fetching the environment as it does mutate the instance final List cmd = self.command(); if (cmd != null && cmd.size() > 0) { - iastCallback(cmd); - raspCallback(cmd); - } - } - - private static void iastCallback(List cmd) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onProcessBuilderStart(cmd); - } catch (final Throwable e) { - module.onUnexpectedException("beforeStart threw", e); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onProcessBuilderStart(cmd); + } catch (final Throwable e) { + module.onUnexpectedException("beforeStart threw", e); + } } } } - - private static void raspCallback(List list) { - ShellCmdRaspHelper.INSTANCE.beforeShellCmd(list.toArray(new String[0])); - } } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java index 7c1983c2cd1..bfde913aff3 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java @@ -23,6 +23,7 @@ public static AgentSpan startSpan(@Advice.Argument(0) final String[] command) th span.setResourceName(ProcessImplInstrumentationHelpers.determineResource(command)); span.setTag("component", "subprocess"); ProcessImplInstrumentationHelpers.setTags(span, command); + ProcessImplInstrumentationHelpers.shiRaspCheck(command); return span; } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java index 102797d7fa9..f6ae37be422 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeCallSite.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.java.lang; import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.appsec.RaspCallSites; import datadog.trace.api.iast.IastCallSites; import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Sink; @@ -11,24 +10,34 @@ import javax.annotation.Nullable; @Sink(VulnerabilityTypes.COMMAND_INJECTION) -@CallSite( - spi = {IastCallSites.class, RaspCallSites.class}, - helpers = ShellCmdRaspHelper.class) +@CallSite(spi = IastCallSites.class) public class RuntimeCallSite { @CallSite.Before("java.lang.Process java.lang.Runtime.exec(java.lang.String)") public static void beforeStart(@CallSite.Argument @Nullable final String command) { if (command != null) { // runtime fails if null - iastCallback(command); - raspCallback(command); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(command); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } } } @CallSite.Before("java.lang.Process java.lang.Runtime.exec(java.lang.String[])") public static void beforeExec(@CallSite.Argument @Nullable final String[] cmdArray) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - iastCallback(cmdArray); - raspCallback(cmdArray); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(cmdArray); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } } } @@ -37,8 +46,14 @@ public static void beforeExec( @CallSite.Argument @Nullable final String command, @CallSite.Argument @Nullable final String[] envp) { if (command != null) { // runtime fails if null - iastCallback(envp, command); - raspCallback(command); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, command); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } } } @@ -48,8 +63,14 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] cmdArray, @CallSite.Argument @Nullable final String[] envp) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - iastCallback(envp, cmdArray); - raspCallback(cmdArray); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, cmdArray); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } } } @@ -60,8 +81,14 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] envp, @CallSite.Argument @Nullable final File dir) { if (command != null) { // runtime fails if null - iastCallback(envp, command); - raspCallback(command); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, command); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } + } } } @@ -72,60 +99,14 @@ public static void beforeExec( @CallSite.Argument @Nullable final String[] envp, @CallSite.Argument @Nullable final File dir) { if (cmdArray != null && cmdArray.length > 0) { // runtime fails if null or empty - iastCallback(envp, cmdArray); - raspCallback(cmdArray); - } - } - - private static void iastCallback(String command) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(command); - } catch (final Throwable e) { - module.onUnexpectedException("iastCallback threw", e); - } - } - } - - private static void iastCallback(String[] cmdArray) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(cmdArray); - } catch (final Throwable e) { - module.onUnexpectedException("iastCallback threw", e); + final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; + if (module != null) { + try { + module.onRuntimeExec(envp, cmdArray); + } catch (final Throwable e) { + module.onUnexpectedException("beforeExec threw", e); + } } } } - - private static void iastCallback(String[] envp, String command) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, command); - } catch (final Throwable e) { - module.onUnexpectedException("iastCallback threw", e); - } - } - } - - private static void iastCallback(String[] envp, String[] cmdArray) { - final CommandInjectionModule module = InstrumentationBridge.COMMAND_INJECTION; - if (module != null) { - try { - module.onRuntimeExec(envp, cmdArray); - } catch (final Throwable e) { - module.onUnexpectedException("iastCallback threw", e); - } - } - } - - private static void raspCallback(String command) { - ShellCmdRaspHelper.INSTANCE.beforeShellCmd(command); - } - - private static void raspCallback(String[] cmdArray) { - ShellCmdRaspHelper.INSTANCE.beforeShellCmd(cmdArray); - } } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java deleted file mode 100644 index 9ed26af5f4d..00000000000 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelper.java +++ /dev/null @@ -1,82 +0,0 @@ -package datadog.trace.instrumentation.java.lang; - -import static datadog.trace.api.gateway.Events.EVENTS; - -import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.Config; -import datadog.trace.api.gateway.BlockResponseFunction; -import datadog.trace.api.gateway.Flow; -import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import java.util.function.BiFunction; -import javax.annotation.Nonnull; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class ShellCmdRaspHelper { - - public static ShellCmdRaspHelper INSTANCE = new ShellCmdRaspHelper(); - - private static final Logger LOGGER = LoggerFactory.getLogger(ShellCmdRaspHelper.class); - - private ShellCmdRaspHelper() { - // prevent instantiation - } - - public void beforeShellCmd(@Nonnull final String[] cmdArray) { - shellCmd(cmdArray); - } - - public void beforeShellCmd(@Nonnull final String cmd) { - shellCmd(cmd); - } - - private void shellCmd(Object cmd) { - if (!Config.get().isAppSecRaspEnabled()) { - return; - } - try { - final BiFunction> shellCmdCallback = - AgentTracer.get() - .getCallbackProvider(RequestContextSlot.APPSEC) - .getCallback(EVENTS.shellCmd()); - - if (shellCmdCallback == null) { - return; - } - - final AgentSpan span = AgentTracer.get().activeSpan(); - if (span == null) { - return; - } - - final RequestContext ctx = span.getRequestContext(); - if (ctx == null) { - return; - } - - Flow flow = shellCmdCallback.apply(ctx, cmd); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = ctx.getBlockResponseFunction(); - if (brf != null) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse( - ctx.getTraceSegment(), - rba.getStatusCode(), - rba.getBlockingContentType(), - rba.getExtraHeaders()); - } - throw new BlockingException("Blocked request (for SHI attempt)"); - } - } catch (final BlockingException e) { - // re-throw blocking exceptions - throw e; - } catch (final Throwable e) { - // suppress anything else - LOGGER.debug("Exception during SHI rasp callback", e); - } - } -} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy similarity index 81% rename from dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy rename to dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy index 27b49c995d4..84014c59872 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ShellCmdRaspHelperForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy @@ -9,13 +9,14 @@ import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.internal.TraceSegment import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.bootstrap.instrumentation.api.java.lang.ProcessImplInstrumentationHelpers import spock.lang.Shared import java.util.function.BiFunction import static datadog.trace.api.gateway.Events.EVENTS -class ShellCmdRaspHelperForkedTest extends AgentTestRunner { +class ProcessImplInstrumentationShellCmdRaspForkedTest extends AgentTestRunner { @Shared protected static final ORIGINAL_TRACER = AgentTracer.get() @@ -49,7 +50,7 @@ class ShellCmdRaspHelperForkedTest extends AgentTestRunner { injectSysConfig(AppSecConfig.APPSEC_RASP_ENABLED, 'true') } - void 'test Helper'() { + void 'test shiRaspCheck'() { setup: final callbackProvider = Mock(CallbackProvider) @@ -58,15 +59,10 @@ class ShellCmdRaspHelperForkedTest extends AgentTestRunner { tracer.getCallbackProvider(RequestContextSlot.APPSEC) >> callbackProvider when: - ShellCmdRaspHelper.INSTANCE.beforeShellCmd(*args) + ProcessImplInstrumentationHelpers.shiRaspCheck(['cat etc/password'] as String[]) then: 1 * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener - 1 * listener.apply(reqCtx, expected) >> flow - - where: - args | expected - ['cat etc/password'] | 'cat etc/password' - [['cat etc/password', 'cat etc/password'] as String[]] | ['cat etc/password', 'cat etc/password'] + 1 * listener.apply(reqCtx, ['cat etc/password']) >> flow } } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index aee0fb550d8..86c57148002 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -573,11 +573,12 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { where: endpoint | cmd | params - 'cmd' | ['cat etc/password'] | null + //TODO these test that receive String cmd instead of String[] cmd are not working due to internally are converted to ["cat", "etc/password"] and the WAF is not blocking them + // 'cmd' | ['cat etc/password'] | null + // 'cmdWithParams' | ['cat etc/password'] | ['param'] + // 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] 'arrayCmd' | ['cat etc/password', 'cat etc/password'] | null - 'cmdWithParams' | ['cat etc/password'] | ['param'] 'arrayCmdWithParams' | ['cat etc/password', 'cat etc/password'] | ['param'] - 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] 'arrayCmdWithParamsAndFile' | ['cat etc/password', 'cat etc/password'] | ['param'] 'processBuilder' | ['cat etc/password', 'cat etc/password'] | null } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index 83c23cb18c3..e01ce935208 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -314,8 +314,8 @@ public EventType("shell.cmd", SHELL_CDM_ID); @SuppressWarnings("unchecked") - public EventType>> shellCmd() { - return (EventType>>) SHELL_CMD; + public EventType>> shellCmd() { + return (EventType>>) SHELL_CMD; } static final int MAX_EVENTS = nextId.get(); diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 816e74b3200..ea1d83dec20 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -434,11 +434,11 @@ public Flow apply(RequestContext ctx, String arg) { }; case SHELL_CDM_ID: return (C) - new BiFunction>() { + new BiFunction>() { @Override - public Flow apply(RequestContext ctx, Object arg) { + public Flow apply(RequestContext ctx, String[] arg) { try { - return ((BiFunction>) callback) + return ((BiFunction>) callback) .apply(ctx, arg); } catch (Throwable t) { log.warn("Callback for {} threw.", eventType, t); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 1d19fccc298..003aac2b01e 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -1,7 +1,14 @@ package datadog.trace.bootstrap.instrumentation.api.java.lang; +import static datadog.trace.api.gateway.Events.EVENTS; import static java.lang.invoke.MethodType.methodType; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.bootstrap.ActiveSubsystems; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -14,10 +21,18 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; +import java.util.function.BiFunction; import java.util.regex.Pattern; +import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** This class is included here because it needs to injected into the bootstrap clasloader. */ public class ProcessImplInstrumentationHelpers { + + private static final Logger LOGGER = + LoggerFactory.getLogger(ProcessImplInstrumentationHelpers.class); + private static final int LIMIT = 4096; public static final boolean ONLINE; private static final MethodHandle PROCESS_ON_EXIT; @@ -185,6 +200,56 @@ public static CharSequence determineResource(String[] command) { return first.substring(pos + 1); } + /* + Check if there is a shell injection attempt to block it + */ + public static void shiRaspCheck(@Nonnull final String[] cmdArray) { + if (!Config.get().isAppSecRaspEnabled()) { + return; + } + try { + final BiFunction> shellCmdCallback = + AgentTracer.get() + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.shellCmd()); + + if (shellCmdCallback == null) { + return; + } + + final AgentSpan span = AgentTracer.get().activeSpan(); + if (span == null) { + return; + } + + final RequestContext ctx = span.getRequestContext(); + if (ctx == null) { + return; + } + + Flow flow = shellCmdCallback.apply(ctx, cmdArray); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + brf.tryCommitBlockingResponse( + ctx.getTraceSegment(), + rba.getStatusCode(), + rba.getBlockingContentType(), + rba.getExtraHeaders()); + } + throw new BlockingException("Blocked request (for SHI attempt)"); + } + } catch (final BlockingException e) { + // re-throw blocking exceptions + throw e; + } catch (final Throwable e) { + // suppress anything else + LOGGER.debug("Exception during SHI rasp callback", e); + } + } + private static AgentScope.Continuation captureContinuation() { final AgentScope parentScope = AgentTracer.activeScope(); return parentScope == null ? null : parentScope.capture(); From 6d971455122188062e59d4990a8f53d0c3e7aa4e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 11 Dec 2024 19:20:24 +0100 Subject: [PATCH 11/20] Change to cmdi keeping ProcessImpl approach --- .../config/AppSecConfigServiceImpl.java | 6 ++-- .../appsec/event/data/KnownAddresses.java | 8 +++--- .../datadog/appsec/gateway/GatewayBridge.java | 18 ++++++------ ...ppSecConfigServiceImplSpecification.groovy | 8 +++--- .../data/KnownAddressesSpecification.groovy | 2 +- .../gateway/GatewayBridgeSpecification.groovy | 14 +++++----- .../java/lang/ProcessImplStartAdvice.java | 2 +- ...strumentationExecCmdRaspForkedTest.groovy} | 10 +++---- .../trace/agent/test/utils/OkHttpUtils.java | 2 +- .../springboot/controller/WebController.java | 28 +++---------------- .../appsec/SpringBootSmokeTest.groovy | 27 ++++++++---------- .../datadog/trace/api/gateway/Events.java | 8 +++--- .../api/gateway/InstrumentationGateway.java | 4 +-- .../datadog/trace/api/telemetry/RuleType.java | 2 +- .../ProcessImplInstrumentationHelpers.java | 16 +++++------ .../gateway/InstrumentationGatewayTest.java | 8 +++--- .../datadog/remoteconfig/Capabilities.java | 2 +- 17 files changed, 71 insertions(+), 94 deletions(-) rename dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/{ProcessImplInstrumentationShellCmdRaspForkedTest.groovy => ProcessImplInstrumentationExecCmdRaspForkedTest.groovy} (82%) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java index 652cb398851..d6ad9b2aa2f 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java @@ -12,8 +12,8 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_HEADER_FINGERPRINT; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT; +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_CMDI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI; -import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING; @@ -119,7 +119,7 @@ private void subscribeConfigurationPoller() { capabilities |= CAPABILITY_ASM_RASP_SQLI; capabilities |= CAPABILITY_ASM_RASP_SSRF; capabilities |= CAPABILITY_ASM_RASP_LFI; - capabilities |= CAPABILITY_ASM_RASP_SHI; + capabilities |= CAPABILITY_ASM_RASP_CMDI; } this.configurationPoller.addCapabilities(capabilities); } @@ -364,7 +364,7 @@ public void close() { | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_LFI - | CAPABILITY_ASM_RASP_SHI + | CAPABILITY_ASM_RASP_CMDI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index a0b2eeba6f0..fefa6c08775 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -131,8 +131,8 @@ public interface KnownAddresses { /** Login success business event */ Address LOGIN_SUCCESS = new Address<>("server.business_logic.users.login.success"); - /** The Shell command being executed */ - Address SHELL_CMD = new Address<>("server.sys.shell.cmd"); + /** The Exec command being executed */ + Address EXEC_CMD = new Address<>("server.sys.exec.cmd"); Address> WAF_CONTEXT_PROCESSOR = new Address<>("waf.context.processor"); @@ -208,8 +208,8 @@ static Address forName(String name) { return LOGIN_SUCCESS; case "server.business_logic.users.login.failure": return LOGIN_FAILURE; - case "server.sys.shell.cmd": - return SHELL_CMD; + case "server.sys.exec.cmd": + return EXEC_CMD; default: return null; } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index d2ebe72dd9f..ebd2414e8d6 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -94,7 +94,7 @@ public class GatewayBridge { private volatile DataSubscriberInfo sessionIdSubInfo; private final ConcurrentHashMap, DataSubscriberInfo> userIdSubInfo = new ConcurrentHashMap<>(); - private volatile DataSubscriberInfo shellCmdSubInfo; + private volatile DataSubscriberInfo execCmdSubInfo; public GatewayBridge( SubscriptionService subscriptionService, @@ -141,7 +141,7 @@ public void init() { EVENTS.loginSuccess(), this.onUserEvent(KnownAddresses.LOGIN_SUCCESS)); subscriptionService.registerCallback( EVENTS.loginFailure(), this.onUserEvent(KnownAddresses.LOGIN_FAILURE)); - subscriptionService.registerCallback(EVENTS.shellCmd(), this::onShellCmd); + subscriptionService.registerCallback(EVENTS.execCmd(), this::onExecCmd); if (additionalIGEvents.contains(EVENTS.requestPathParams())) { subscriptionService.registerCallback(EVENTS.requestPathParams(), this::onRequestPathParams); @@ -260,27 +260,27 @@ private Flow onNetworkConnection(RequestContext ctx_, String url) { } } - private Flow onShellCmd(RequestContext ctx_, String[] command) { + private Flow onExecCmd(RequestContext ctx_, String[] command) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { return NoopFlow.INSTANCE; } while (true) { - DataSubscriberInfo subInfo = shellCmdSubInfo; + DataSubscriberInfo subInfo = execCmdSubInfo; if (subInfo == null) { - subInfo = producerService.getDataSubscribers(KnownAddresses.SHELL_CMD); - shellCmdSubInfo = subInfo; + subInfo = producerService.getDataSubscribers(KnownAddresses.EXEC_CMD); + execCmdSubInfo = subInfo; } if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } DataBundle bundle = - new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.SHELL_CMD, command).build(); + new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.EXEC_CMD, command).build(); try { - GatewayContext gwCtx = new GatewayContext(true, RuleType.SHI); + GatewayContext gwCtx = new GatewayContext(true, RuleType.COMMAND_INJECTION); return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); } catch (ExpiredSubscriberInfoException e) { - shellCmdSubInfo = null; + execCmdSubInfo = null; } } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy index 670714cab04..9bf2ef4af80 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy @@ -27,7 +27,7 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_HEADER_FINGERPRIN import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI -import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_CMDI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING @@ -272,7 +272,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_TRUSTED_IPS | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF - | CAPABILITY_ASM_RASP_SHI + | CAPABILITY_ASM_RASP_CMDI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -425,7 +425,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_TRUSTED_IPS | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF - | CAPABILITY_ASM_RASP_SHI + | CAPABILITY_ASM_RASP_CMDI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -499,7 +499,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_API_SECURITY_SAMPLE_RATE | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF - | CAPABILITY_ASM_RASP_SHI + | CAPABILITY_ASM_RASP_CMDI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy index a3e886efce4..1bf30df94ec 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy @@ -41,7 +41,7 @@ class KnownAddressesSpecification extends Specification { 'server.business_logic.users.login.success', 'server.io.net.url', 'server.io.fs.file', - 'server.sys.shell.cmd', + 'server.sys.exec.cmd', 'waf.context.processor' ] } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index f1503f5d6a8..17fea5d29fe 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -92,7 +92,7 @@ class GatewayBridgeSpecification extends DDSpecification { TriFunction> userIdCB TriFunction> loginSuccessCB TriFunction> loginFailureCB - BiFunction> shellCmdCB + BiFunction> execCmdCB void setup() { callInitAndCaptureCBs() @@ -432,7 +432,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * ig.registerCallback(EVENTS.userId(), _) >> { userIdCB = it[1]; null } 1 * ig.registerCallback(EVENTS.loginSuccess(), _) >> { loginSuccessCB = it[1]; null } 1 * ig.registerCallback(EVENTS.loginFailure(), _) >> { loginFailureCB = it[1]; null } - 1 * ig.registerCallback(EVENTS.shellCmd(), _) >> { shellCmdCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.execCmd(), _) >> { execCmdCB = it[1]; null } 0 * ig.registerCallback(_, _) bridge.init() @@ -835,20 +835,20 @@ class GatewayBridgeSpecification extends DDSpecification { gatewayContext.isRasp == true } - void 'process shell cmd'() { + void 'process exec cmd'() { setup: - final cmd = '<!--#exec%20cmd="/bin/cat%20/etc/passwd"-->' - eventDispatcher.getDataSubscribers({ KnownAddresses.SHELL_CMD in it }) >> nonEmptyDsInfo + final cmd = ['/bin/../usr/bin/reboot', '-f'] as String[] + eventDispatcher.getDataSubscribers({ KnownAddresses.EXEC_CMD in it }) >> nonEmptyDsInfo DataBundle bundle GatewayContext gatewayContext when: - Flow flow = shellCmdCB.apply(ctx, cmd) + Flow flow = execCmdCB.apply(ctx, cmd) then: 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE } - bundle.get(KnownAddresses.SHELL_CMD) == cmd + bundle.get(KnownAddresses.EXEC_CMD) == cmd flow.result == null flow.action == Flow.Action.Noop.INSTANCE gatewayContext.isTransient == true diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java index bfde913aff3..028f4f22eea 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java @@ -23,7 +23,7 @@ public static AgentSpan startSpan(@Advice.Argument(0) final String[] command) th span.setResourceName(ProcessImplInstrumentationHelpers.determineResource(command)); span.setTag("component", "subprocess"); ProcessImplInstrumentationHelpers.setTags(span, command); - ProcessImplInstrumentationHelpers.shiRaspCheck(command); + ProcessImplInstrumentationHelpers.cmdiRaspCheck(command); return span; } diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationExecCmdRaspForkedTest.groovy similarity index 82% rename from dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy rename to dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationExecCmdRaspForkedTest.groovy index 84014c59872..1746329f2ee 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationShellCmdRaspForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationExecCmdRaspForkedTest.groovy @@ -16,7 +16,7 @@ import java.util.function.BiFunction import static datadog.trace.api.gateway.Events.EVENTS -class ProcessImplInstrumentationShellCmdRaspForkedTest extends AgentTestRunner { +class ProcessImplInstrumentationExecCmdRaspForkedTest extends AgentTestRunner { @Shared protected static final ORIGINAL_TRACER = AgentTracer.get() @@ -50,7 +50,7 @@ class ProcessImplInstrumentationShellCmdRaspForkedTest extends AgentTestRunner injectSysConfig(AppSecConfig.APPSEC_RASP_ENABLED, 'true') } - void 'test shiRaspCheck'() { + void 'test cmdiRaspCheck'() { setup: final callbackProvider = Mock(CallbackProvider) @@ -59,10 +59,10 @@ class ProcessImplInstrumentationShellCmdRaspForkedTest extends AgentTestRunner tracer.getCallbackProvider(RequestContextSlot.APPSEC) >> callbackProvider when: - ProcessImplInstrumentationHelpers.shiRaspCheck(['cat etc/password'] as String[]) + ProcessImplInstrumentationHelpers.cmdiRaspCheck(['/bin/../usr/bin/reboot', '-f'] as String[]) then: - 1 * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener - 1 * listener.apply(reqCtx, ['cat etc/password']) >> flow + 1 * callbackProvider.getCallback(EVENTS.execCmd()) >> listener + 1 * listener.apply(reqCtx, ['/bin/../usr/bin/reboot', '-f']) >> flow } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java index 361aab3845c..d93ad269cc2 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java @@ -52,7 +52,7 @@ public class OkHttpUtils { } public static OkHttpClient.Builder clientBuilder() { - final TimeUnit unit = TimeUnit.MINUTES; + final TimeUnit unit = TimeUnit.HOURS; return new OkHttpClient.Builder() .addInterceptor(EXPECT_CONTINUE_INTERCEPTOR) .addInterceptor(LOGGING_INTERCEPTOR) diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 1a590fca21d..577efe7a086 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -159,47 +159,27 @@ public ResponseEntity session(final HttpServletRequest request) { return new ResponseEntity<>(session.getId(), HttpStatus.OK); } - @PostMapping("/shi/arrayCmd") + @PostMapping("/cmdi/arrayCmd") public String shiArrayCmd(@RequestParam("cmd") String[] arrayCmd) { withProcess(() -> Runtime.getRuntime().exec(arrayCmd)); return "EXECUTED"; } - @PostMapping("/shi/cmd") - public String shiCmd(@RequestParam("cmd") String cmd) { - withProcess(() -> Runtime.getRuntime().exec(cmd)); - return "EXECUTED"; - } - - @PostMapping("/shi/arrayCmdWithParams") + @PostMapping("/cmdi/arrayCmdWithParams") public String shiArrayCmdWithParams( @RequestParam("cmd") String[] arrayCmd, @RequestParam("params") String[] params) { withProcess(() -> Runtime.getRuntime().exec(arrayCmd, params)); return "EXECUTED"; } - @PostMapping("/shi/cmdWithParams") - public String shiCmdWithParams( - @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { - withProcess(() -> Runtime.getRuntime().exec(cmd, params)); - return "EXECUTED"; - } - - @PostMapping("/shi/arrayCmdWithParamsAndFile") + @PostMapping("/cmdi/arrayCmdWithParamsAndFile") public String shiArrayCmdWithParamsAndFile( @RequestParam("cmd") String[] arrayCmd, @RequestParam("params") String[] params) { withProcess(() -> Runtime.getRuntime().exec(arrayCmd, params, new File(""))); return "EXECUTED"; } - @PostMapping("/shi/cmdParamsAndFile") - public String shiCmdParamsAndFile( - @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { - withProcess(() -> Runtime.getRuntime().exec(cmd, params, new File(""))); - return "EXECUTED"; - } - - @PostMapping("/shi/processBuilder") + @PostMapping("/cmdi/processBuilder") public String shiProcessBuilder(@RequestParam("cmd") String[] cmd) { withProcess(() -> new ProcessBuilder(cmd).start()); return "EXECUTED"; diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 86c57148002..8b25e971860 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -153,8 +153,8 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { on_match : ['block'] ], [ - id : '__test_shi_block', - name : 'Shell injection exploit', + id : 'rasp-932-100', // to replace default rule + name : 'Command injection exploit', enable : 'true', tags : [ type: 'command_injection', @@ -167,10 +167,10 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { conditions : [ [ parameters: [ - resource: [[address: 'server.sys.shell.cmd']], + resource: [[address: 'server.sys.exec.cmd']], params : [[address: 'server.request.body']], ], - operator : "shi_detector", + operator : "cmdi_detector", ], ], transformers: [], @@ -529,9 +529,9 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { assert trigger != null, 'test trigger not found' } - void 'rasp blocks on SHI'() { + void 'rasp blocks on CMDI'() { when: - String url = "http://localhost:${httpPort}/shi/"+endpoint + String url = "http://localhost:${httpPort}/cmdi/"+endpoint def formBuilder = new FormBody.Builder() for (s in cmd) { formBuilder.add("cmd", s) @@ -564,7 +564,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { assert rootSpan.meta.get('_dd.appsec.json') != null, '_dd.appsec.json is not set' def trigger = null for (t in rootSpan.triggers) { - if (t['rule']['id'] == '__test_shi_block') { + if (t['rule']['id'] == 'rasp-932-100') { trigger = t break } @@ -573,14 +573,11 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { where: endpoint | cmd | params - //TODO these test that receive String cmd instead of String[] cmd are not working due to internally are converted to ["cat", "etc/password"] and the WAF is not blocking them - // 'cmd' | ['cat etc/password'] | null - // 'cmdWithParams' | ['cat etc/password'] | ['param'] - // 'cmdParamsAndFile' | ['cat etc/password'] | ['param'] - 'arrayCmd' | ['cat etc/password', 'cat etc/password'] | null - 'arrayCmdWithParams' | ['cat etc/password', 'cat etc/password'] | ['param'] - 'arrayCmdWithParamsAndFile' | ['cat etc/password', 'cat etc/password'] | ['param'] - 'processBuilder' | ['cat etc/password', 'cat etc/password'] | null + 'arrayCmd' | ['/bin/evilCommand'] | null + 'arrayCmdWithParams' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] + 'arrayCmdWithParamsAndFile' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] + 'processBuilder' | ['/bin/evilCommand'] | null + } } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index e01ce935208..ccb3ba2d083 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -308,14 +308,14 @@ public EventType("shell.cmd", SHELL_CDM_ID); + private static final EventType EXEC_CMD = new ET<>("exec.cmd", EXEC_CMD_ID); @SuppressWarnings("unchecked") - public EventType>> shellCmd() { - return (EventType>>) SHELL_CMD; + public EventType>> execCmd() { + return (EventType>>) EXEC_CMD; } static final int MAX_EVENTS = nextId.get(); diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index ea1d83dec20..6c67985383f 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -2,6 +2,7 @@ import static datadog.trace.api.gateway.Events.DATABASE_CONNECTION_ID; import static datadog.trace.api.gateway.Events.DATABASE_SQL_QUERY_ID; +import static datadog.trace.api.gateway.Events.EXEC_CMD_ID; import static datadog.trace.api.gateway.Events.FILE_LOADED_ID; import static datadog.trace.api.gateway.Events.GRAPHQL_SERVER_REQUEST_MESSAGE_ID; import static datadog.trace.api.gateway.Events.GRPC_SERVER_METHOD_ID; @@ -25,7 +26,6 @@ import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_DONE_ID; import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_ID; import static datadog.trace.api.gateway.Events.RESPONSE_STARTED_ID; -import static datadog.trace.api.gateway.Events.SHELL_CDM_ID; import static datadog.trace.api.gateway.Events.USER_ID; import datadog.trace.api.UserIdCollectionMode; @@ -432,7 +432,7 @@ public Flow apply(RequestContext ctx, String arg) { } } }; - case SHELL_CDM_ID: + case EXEC_CMD_ID: return (C) new BiFunction>() { @Override diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java index bae7501122b..6b9217138f4 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java @@ -4,7 +4,7 @@ public enum RuleType { LFI("lfi"), SQL_INJECTION("sql_injection"), SSRF("ssrf"), - SHI("shi"); + COMMAND_INJECTION("command_injection"); public final String name; private static final int numValues = RuleType.values().length; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 003aac2b01e..7f0e6a2b14f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -201,19 +201,19 @@ public static CharSequence determineResource(String[] command) { } /* - Check if there is a shell injection attempt to block it + Check if there is a cmd injection attempt to block it */ - public static void shiRaspCheck(@Nonnull final String[] cmdArray) { + public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { if (!Config.get().isAppSecRaspEnabled()) { return; } try { - final BiFunction> shellCmdCallback = + final BiFunction> execCmdCallback = AgentTracer.get() .getCallbackProvider(RequestContextSlot.APPSEC) - .getCallback(EVENTS.shellCmd()); + .getCallback(EVENTS.execCmd()); - if (shellCmdCallback == null) { + if (execCmdCallback == null) { return; } @@ -227,7 +227,7 @@ public static void shiRaspCheck(@Nonnull final String[] cmdArray) { return; } - Flow flow = shellCmdCallback.apply(ctx, cmdArray); + Flow flow = execCmdCallback.apply(ctx, cmdArray); Flow.Action action = flow.getAction(); if (action instanceof Flow.Action.RequestBlockingAction) { BlockResponseFunction brf = ctx.getBlockResponseFunction(); @@ -239,14 +239,14 @@ public static void shiRaspCheck(@Nonnull final String[] cmdArray) { rba.getBlockingContentType(), rba.getExtraHeaders()); } - throw new BlockingException("Blocked request (for SHI attempt)"); + throw new BlockingException("Blocked request (for CMDI attempt)"); } } catch (final BlockingException e) { // re-throw blocking exceptions throw e; } catch (final Throwable e) { // suppress anything else - LOGGER.debug("Exception during SHI rasp callback", e); + LOGGER.debug("Exception during CMDI rasp callback", e); } } diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index 85527f55181..e6cd66fb7d7 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -221,8 +221,8 @@ public void testNormalCalls() { cbp.getCallback(events.loginSuccess()).apply(null, null, null); ss.registerCallback(events.loginFailure(), callback); cbp.getCallback(events.loginFailure()).apply(null, null, null); - ss.registerCallback(events.shellCmd(), callback); - cbp.getCallback(events.shellCmd()).apply(null, null); + ss.registerCallback(events.execCmd(), callback); + cbp.getCallback(events.execCmd()).apply(null, null); assertThat(callback.count).isEqualTo(Events.MAX_EVENTS); } @@ -291,8 +291,8 @@ public void testThrowableBlocking() { cbp.getCallback(events.loginSuccess()).apply(null, null, null); ss.registerCallback(events.loginFailure(), throwback); cbp.getCallback(events.loginFailure()).apply(null, null, null); - ss.registerCallback(events.shellCmd(), throwback); - cbp.getCallback(events.shellCmd()).apply(null, null); + ss.registerCallback(events.execCmd(), throwback); + cbp.getCallback(events.execCmd()).apply(null, null); assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS); } diff --git a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java index 079ec1a5f81..3c5920e2543 100644 --- a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java +++ b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java @@ -25,7 +25,7 @@ public interface Capabilities { long CAPABILITY_ASM_RASP_SQLI = 1 << 21; long CAPABILITY_ASM_RASP_LFI = 1 << 22; long CAPABILITY_ASM_RASP_SSRF = 1 << 23; - long CAPABILITY_ASM_RASP_SHI = 1 << 24; + long CAPABILITY_ASM_RASP_CMDI = 1 << 24; long CAPABILITY_ASM_RASP_XXE = 1 << 25; long CAPABILITY_ASM_RASP_RCE = 1 << 26; long CAPABILITY_ASM_RASP_NOSQLI = 1 << 27; From 2c7672eec2eb3a3dd92d21dc4ef39a79e2c18c4f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Dec 2024 09:08:42 +0100 Subject: [PATCH 12/20] fix --- .../main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java index d93ad269cc2..361aab3845c 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.java @@ -52,7 +52,7 @@ public class OkHttpUtils { } public static OkHttpClient.Builder clientBuilder() { - final TimeUnit unit = TimeUnit.HOURS; + final TimeUnit unit = TimeUnit.MINUTES; return new OkHttpClient.Builder() .addInterceptor(EXPECT_CONTINUE_INTERCEPTOR) .addInterceptor(LOGGING_INTERCEPTOR) From f3c4fe136be1e9d482ef441eb142ab158a25c18e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Dec 2024 12:10:08 +0100 Subject: [PATCH 13/20] Add SHI --- .../bytebuddy/matcher/ignored_class_name.trie | 2 + .../config/AppSecConfigServiceImpl.java | 3 + .../appsec/event/data/KnownAddresses.java | 5 ++ .../datadog/appsec/gateway/GatewayBridge.java | 27 ++++++ ...ppSecConfigServiceImplSpecification.groovy | 4 + .../data/KnownAddressesSpecification.groovy | 3 +- .../gateway/GatewayBridgeSpecification.groovy | 22 +++++ .../java/lang/RuntimeExecStringAdvice.java | 20 +++++ .../java/lang/RuntimeInstrumentation.java | 37 +++++++++ .../RuntimeInstrumentationForkedTest.groovy | 81 ++++++++++++++++++ .../springboot/controller/WebController.java | 20 +++++ .../appsec/SpringBootSmokeTest.groovy | 83 +++++++++++++++++-- .../datadog/trace/api/gateway/Events.java | 10 +++ .../api/gateway/InstrumentationGateway.java | 2 + .../ProcessImplInstrumentationHelpers.java | 63 ++++++++++++++ .../gateway/InstrumentationGatewayTest.java | 4 + .../datadog/remoteconfig/Capabilities.java | 3 +- 17 files changed, 382 insertions(+), 7 deletions(-) create mode 100644 dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeExecStringAdvice.java create mode 100644 dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java create mode 100644 dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy diff --git a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie index 9372507c6b9..bce733ae3d7 100644 --- a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie +++ b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie @@ -47,6 +47,8 @@ 0 java.lang.Error # allow ProcessImpl instrumentation 0 java.lang.ProcessImpl +# allow Runtime instrumentation for RASP +0 java.lang.Runtime 0 java.net.http.* 0 java.net.HttpURLConnection 0 java.net.Socket diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java index d6ad9b2aa2f..80c7956ea25 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java @@ -14,6 +14,7 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_CMDI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI; +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF; import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING; @@ -120,6 +121,7 @@ private void subscribeConfigurationPoller() { capabilities |= CAPABILITY_ASM_RASP_SSRF; capabilities |= CAPABILITY_ASM_RASP_LFI; capabilities |= CAPABILITY_ASM_RASP_CMDI; + capabilities |= CAPABILITY_ASM_RASP_SHI; } this.configurationPoller.addCapabilities(capabilities); } @@ -365,6 +367,7 @@ public void close() { | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ASM_RASP_CMDI + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index fefa6c08775..7b2f3bdd4e1 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -134,6 +134,9 @@ public interface KnownAddresses { /** The Exec command being executed */ Address EXEC_CMD = new Address<>("server.sys.exec.cmd"); + /** The Shell command being executed */ + Address SHELL_CMD = new Address<>("server.sys.shell.cmd"); + Address> WAF_CONTEXT_PROCESSOR = new Address<>("waf.context.processor"); static Address forName(String name) { @@ -210,6 +213,8 @@ static Address forName(String name) { return LOGIN_FAILURE; case "server.sys.exec.cmd": return EXEC_CMD; + case "server.sys.shell.cmd": + return SHELL_CMD; default: return null; } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index ebd2414e8d6..fb4aec27e0b 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -95,6 +95,7 @@ public class GatewayBridge { private final ConcurrentHashMap, DataSubscriberInfo> userIdSubInfo = new ConcurrentHashMap<>(); private volatile DataSubscriberInfo execCmdSubInfo; + private volatile DataSubscriberInfo shellCmdSubInfo; public GatewayBridge( SubscriptionService subscriptionService, @@ -142,6 +143,7 @@ public void init() { subscriptionService.registerCallback( EVENTS.loginFailure(), this.onUserEvent(KnownAddresses.LOGIN_FAILURE)); subscriptionService.registerCallback(EVENTS.execCmd(), this::onExecCmd); + subscriptionService.registerCallback(EVENTS.shellCmd(), this::onShellCmd); if (additionalIGEvents.contains(EVENTS.requestPathParams())) { subscriptionService.registerCallback(EVENTS.requestPathParams(), this::onRequestPathParams); @@ -285,6 +287,31 @@ private Flow onExecCmd(RequestContext ctx_, String[] command) { } } + private Flow onShellCmd(RequestContext ctx_, String command) { + AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); + if (ctx == null) { + return NoopFlow.INSTANCE; + } + while (true) { + DataSubscriberInfo subInfo = shellCmdSubInfo; + if (subInfo == null) { + subInfo = producerService.getDataSubscribers(KnownAddresses.SHELL_CMD); + shellCmdSubInfo = subInfo; + } + if (subInfo == null || subInfo.isEmpty()) { + return NoopFlow.INSTANCE; + } + DataBundle bundle = + new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.SHELL_CMD, command).build(); + try { + GatewayContext gwCtx = new GatewayContext(true, RuleType.COMMAND_INJECTION); + return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); + } catch (ExpiredSubscriberInfoException e) { + shellCmdSubInfo = null; + } + } + } + private Flow onFileLoaded(RequestContext ctx_, String path) { AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy index 9bf2ef4af80..207f07cf521 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy @@ -28,6 +28,7 @@ import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_IP_BLOCKING import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_NETWORK_FINGERPRINT import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_LFI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_CMDI +import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SHI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SQLI import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_RASP_SSRF import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_REQUEST_BLOCKING @@ -273,6 +274,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_CMDI + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -426,6 +428,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_CMDI + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ENDPOINT_FINGERPRINT | CAPABILITY_ASM_SESSION_FINGERPRINT @@ -500,6 +503,7 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { | CAPABILITY_ASM_RASP_SQLI | CAPABILITY_ASM_RASP_SSRF | CAPABILITY_ASM_RASP_CMDI + | CAPABILITY_ASM_RASP_SHI | CAPABILITY_ASM_RASP_LFI | CAPABILITY_ASM_AUTO_USER_INSTRUM_MODE | CAPABILITY_ENDPOINT_FINGERPRINT diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy index 1bf30df94ec..5ee1776db7a 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy @@ -42,13 +42,14 @@ class KnownAddressesSpecification extends Specification { 'server.io.net.url', 'server.io.fs.file', 'server.sys.exec.cmd', + 'server.sys.shell.cmd', 'waf.context.processor' ] } void 'number of known addresses is expected number'() { expect: - Address.instanceCount() == 36 + Address.instanceCount() == 37 KnownAddresses.WAF_CONTEXT_PROCESSOR.serial == Address.instanceCount() - 1 } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 17fea5d29fe..cdad0461d7a 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -93,6 +93,7 @@ class GatewayBridgeSpecification extends DDSpecification { TriFunction> loginSuccessCB TriFunction> loginFailureCB BiFunction> execCmdCB + BiFunction> shellCmdCB void setup() { callInitAndCaptureCBs() @@ -433,6 +434,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * ig.registerCallback(EVENTS.loginSuccess(), _) >> { loginSuccessCB = it[1]; null } 1 * ig.registerCallback(EVENTS.loginFailure(), _) >> { loginFailureCB = it[1]; null } 1 * ig.registerCallback(EVENTS.execCmd(), _) >> { execCmdCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.shellCmd(), _) >> { shellCmdCB = it[1]; null } 0 * ig.registerCallback(_, _) bridge.init() @@ -855,6 +857,26 @@ class GatewayBridgeSpecification extends DDSpecification { gatewayContext.isRasp == true } + void 'process shell cmd'() { + setup: + final cmd = '$(cat /etc/passwd 1>&2 ; echo .)' + eventDispatcher.getDataSubscribers({ KnownAddresses.SHELL_CMD in it }) >> nonEmptyDsInfo + DataBundle bundle + GatewayContext gatewayContext + + when: + Flow flow = shellCmdCB.apply(ctx, cmd) + + then: + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> + { a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE } + bundle.get(KnownAddresses.SHELL_CMD) == cmd + flow.result == null + flow.action == Flow.Action.Noop.INSTANCE + gatewayContext.isTransient == true + gatewayContext.isRasp == true + } + void 'calls trace segment post processor'() { setup: AgentSpan span = Stub() diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeExecStringAdvice.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeExecStringAdvice.java new file mode 100644 index 00000000000..c8b24590609 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeExecStringAdvice.java @@ -0,0 +1,20 @@ +package datadog.trace.instrumentation.java.lang; + +import datadog.trace.bootstrap.instrumentation.api.java.lang.ProcessImplInstrumentationHelpers; +import java.io.IOException; +import net.bytebuddy.asm.Advice; + +class RuntimeExecStringAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void beforeExec(@Advice.Argument(0) final String command) throws IOException { + if (command == null) { + return; + } + ProcessImplInstrumentationHelpers.shiRaspCheck(command); + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void afterExec() { + ProcessImplInstrumentationHelpers.resetCheckShi(); + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java new file mode 100644 index 00000000000..6b91b6129c9 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.java.lang; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Platform; +import java.io.File; + +@AutoService(InstrumenterModule.class) +public class RuntimeInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.ForBootstrap { + + public RuntimeInstrumentation() { + super("java-lang-appsec"); + } + + @Override + protected boolean defaultEnabled() { + return super.defaultEnabled() + && !Platform.isNativeImageBuilder(); // not applicable in native-image + } + + @Override + public String instrumentedType() { + return "java.lang.Runtime"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("exec").and(takesArguments(String.class, String[].class, File.class)), + packageName + ".RuntimeExecStringAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy new file mode 100644 index 00000000000..86fabb46177 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy @@ -0,0 +1,81 @@ +package datadog.trace.instrumentation.java.lang + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.config.AppSecConfig +import datadog.trace.api.gateway.CallbackProvider +import static datadog.trace.api.gateway.Events.EVENTS +import datadog.trace.api.gateway.Flow +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.internal.TraceSegment +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import spock.lang.Shared + +import java.util.function.BiFunction + +class RuntimeInstrumentationForkedTest extends AgentTestRunner{ + + @Shared + protected static final ORIGINAL_TRACER = AgentTracer.get() + + protected traceSegment + protected reqCtx + protected span + protected tracer + + void setup() { + traceSegment = Stub(TraceSegment) + reqCtx = Stub(RequestContext) { + getTraceSegment() >> traceSegment + } + span = Stub(AgentSpan) { + getRequestContext() >> reqCtx + } + tracer = Stub(AgentTracer.TracerAPI) { + activeSpan() >> span + } + AgentTracer.forceRegister(tracer) + } + + void cleanup() { + AgentTracer.forceRegister(ORIGINAL_TRACER) + } + + @Override + protected void configurePreAgent() { + injectSysConfig(AppSecConfig.APPSEC_ENABLED, 'true') + injectSysConfig(AppSecConfig.APPSEC_RASP_ENABLED, 'true') + } + + void 'test shiRaspCheck'() { + + setup: + final callbackProvider = Mock(CallbackProvider) + final listener = Mock(BiFunction) + final flow = Mock(Flow) + tracer.getCallbackProvider(RequestContextSlot.APPSEC) >> callbackProvider + + when: + try { + Runtime.getRuntime().exec(*args) + }catch (Exception e){ + // ignore + } + + + then: + cmdiExpected * callbackProvider.getCallback(EVENTS.execCmd()) >> listener + shiExpected * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener + 1 * listener.apply(reqCtx, args[0]) >> flow + + where: + args | cmdiExpected | shiExpected + ['$(cat /etc/passwd 1>&2 ; echo .)'] | 0 | 1 + ['$(cat /etc/passwd 1>&2 ; echo .)', ['test'] as String[]] | 0 | 1 + ['$(cat /etc/passwd 1>&2 ; echo .)', ['test'] as String[], new File('')] | 0 | 1 + [['/bin/evilCommand'] as String[]] | 1 | 0 + [['/bin/evilCommand'] as String[], ['test'] as String[]] | 1 | 0 + [['/bin/evilCommand'] as String[], ['test'] as String[], new File('')] | 1 | 0 + } +} diff --git a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java index 577efe7a086..8529a048fbd 100644 --- a/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java +++ b/dd-smoke-tests/appsec/springboot/src/main/java/datadog/smoketest/appsec/springboot/controller/WebController.java @@ -185,6 +185,26 @@ public String shiProcessBuilder(@RequestParam("cmd") String[] cmd) { return "EXECUTED"; } + @PostMapping("/shi/cmd") + public String shiCmd(@RequestParam("cmd") String cmd) { + withProcess(() -> Runtime.getRuntime().exec(cmd)); + return "EXECUTED"; + } + + @PostMapping("/shi/cmdWithParams") + public String shiCmdWithParams( + @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(cmd, params)); + return "EXECUTED"; + } + + @PostMapping("/shi/cmdParamsAndFile") + public String shiCmdParamsAndFile( + @RequestParam("cmd") String cmd, @RequestParam("params") String[] params) { + withProcess(() -> Runtime.getRuntime().exec(cmd, params, new File(""))); + return "EXECUTED"; + } + private void withProcess(final Operation op) { Process process = null; try { diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 8b25e971860..d44da4c93fc 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -153,7 +153,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { on_match : ['block'] ], [ - id : 'rasp-932-100', // to replace default rule + id : 'rasp-932-110', // to replace default rule name : 'Command injection exploit', enable : 'true', tags : [ @@ -175,6 +175,30 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { ], transformers: [], on_match : ['block'] + ], + [ + id : 'rasp-932-100', // to replace default rule + name : 'Shell command injection exploit', + enable : 'true', + tags : [ + type: 'command_injection', + category: 'vulnerability_trigger', + cwe: '77', + capec: '1000/152/248/88', + confidence: '0', + module: 'rasp' + ], + conditions : [ + [ + parameters: [ + resource: [[address: 'server.sys.shell.cmd']], + params : [[address: 'server.request.body']], + ], + operator : "shi_detector", + ], + ], + transformers: [], + on_match : ['block'] ] ]) } @@ -564,7 +588,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { assert rootSpan.meta.get('_dd.appsec.json') != null, '_dd.appsec.json is not set' def trigger = null for (t in rootSpan.triggers) { - if (t['rule']['id'] == 'rasp-932-100') { + if (t['rule']['id'] == 'rasp-932-110') { trigger = t break } @@ -572,11 +596,60 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { assert trigger != null, 'test trigger not found' where: - endpoint | cmd | params - 'arrayCmd' | ['/bin/evilCommand'] | null + endpoint | cmd | params + 'arrayCmd' | ['/bin/evilCommand'] | null 'arrayCmdWithParams' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] 'arrayCmdWithParamsAndFile' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] - 'processBuilder' | ['/bin/evilCommand'] | null + 'processBuilder' | ['/bin/evilCommand'] | null + } + + void 'rasp blocks on SHI'() { + when: + String url = "http://localhost:${httpPort}/shi/"+endpoint + def formBuilder = new FormBody.Builder() + for (s in cmd) { + formBuilder.add("cmd", s) + } + if (params != null) { + for (s in params) { + formBuilder.add("params", s) + } + } + final body = formBuilder.build() + def request = new Request.Builder() + .url(url) + .post(body) + .build() + def response = client.newCall(request).execute() + def responseBodyStr = response.body().string() + + then: + response.code() == 403 + responseBodyStr.contains('You\'ve been blocked') + + when: + waitForTraceCount(1) + + then: + def rootSpans = this.rootSpans.toList() + rootSpans.size() == 1 + def rootSpan = rootSpans[0] + assert rootSpan.meta.get('appsec.blocked') == 'true', 'appsec.blocked is not set' + assert rootSpan.meta.get('_dd.appsec.json') != null, '_dd.appsec.json is not set' + def trigger = null + for (t in rootSpan.triggers) { + if (t['rule']['id'] == 'rasp-932-100') { + trigger = t + break + } + } + assert trigger != null, 'test trigger not found' + + where: + endpoint | cmd | params + 'cmd' | ['$(cat /etc/passwd 1>&2 ; echo .)'] | null + 'cmdWithParams' | ['$(cat /etc/passwd 1>&2 ; echo .)'] | ['param'] + 'cmdParamsAndFile' | ['$(cat /etc/passwd 1>&2 ; echo .)'] | ['param'] } diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index ccb3ba2d083..fa123118e48 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -318,6 +318,16 @@ public EventType>> execCmd() { return (EventType>>) EXEC_CMD; } + static final int SHELL_CMD_ID = 26; + + @SuppressWarnings("rawtypes") + private static final EventType SHELL_CMD = new ET<>("shell.cmd", SHELL_CMD_ID); + + @SuppressWarnings("unchecked") + public EventType>> shellCmd() { + return (EventType>>) SHELL_CMD; + } + static final int MAX_EVENTS = nextId.get(); private static final class ET extends EventType { diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 6c67985383f..aa6d5811c95 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -26,6 +26,7 @@ import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_DONE_ID; import static datadog.trace.api.gateway.Events.RESPONSE_HEADER_ID; import static datadog.trace.api.gateway.Events.RESPONSE_STARTED_ID; +import static datadog.trace.api.gateway.Events.SHELL_CMD_ID; import static datadog.trace.api.gateway.Events.USER_ID; import datadog.trace.api.UserIdCollectionMode; @@ -419,6 +420,7 @@ public Flow apply(RequestContext ctx, String arg) { case DATABASE_SQL_QUERY_ID: case NETWORK_CONNECTION_ID: case FILE_LOADED_ID: + case SHELL_CMD_ID: return (C) new BiFunction>() { @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 7f0e6a2b14f..7289de437a6 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -44,6 +44,10 @@ public class ProcessImplInstrumentationHelpers { + "a(?:ccess|uth)_token|mysql_pwd|credentials|(?:stripe)?token)$"); private static final Set REDACTED_BINARIES = Collections.singleton("md5"); + // This check is used to avoid command injection exploit prevention if shell injection exploit + // prevention checked + private static final ThreadLocal checkShi = ThreadLocal.withInitial(() -> false); + static { MethodHandle processOnExit = null; Executor executor = null; @@ -207,6 +211,10 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { if (!Config.get().isAppSecRaspEnabled()) { return; } + // if shell injection was checked, skip cmd injection check + if (checkShi.get()) { + return; + } try { final BiFunction> execCmdCallback = AgentTracer.get() @@ -250,6 +258,61 @@ public static void cmdiRaspCheck(@Nonnull final String[] cmdArray) { } } + public static void resetCheckShi() { + checkShi.set(false); + } + + /* + Check if there is a cmd injection attempt to block it + */ + public static void shiRaspCheck(@Nonnull final String cmd) { + if (!Config.get().isAppSecRaspEnabled()) { + return; + } + checkShi.set(true); + try { + final BiFunction> shellCmdCallback = + AgentTracer.get() + .getCallbackProvider(RequestContextSlot.APPSEC) + .getCallback(EVENTS.shellCmd()); + + if (shellCmdCallback == null) { + return; + } + + final AgentSpan span = AgentTracer.get().activeSpan(); + if (span == null) { + return; + } + + final RequestContext ctx = span.getRequestContext(); + if (ctx == null) { + return; + } + + Flow flow = shellCmdCallback.apply(ctx, cmd); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + brf.tryCommitBlockingResponse( + ctx.getTraceSegment(), + rba.getStatusCode(), + rba.getBlockingContentType(), + rba.getExtraHeaders()); + } + throw new BlockingException("Blocked request (for SHI attempt)"); + } + } catch (final BlockingException e) { + // re-throw blocking exceptions + throw e; + } catch (final Throwable e) { + // suppress anything else + LOGGER.debug("Exception during SHI rasp callback", e); + } + } + private static AgentScope.Continuation captureContinuation() { final AgentScope parentScope = AgentTracer.activeScope(); return parentScope == null ? null : parentScope.capture(); diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index e6cd66fb7d7..65c9bfa874a 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -223,6 +223,8 @@ public void testNormalCalls() { cbp.getCallback(events.loginFailure()).apply(null, null, null); ss.registerCallback(events.execCmd(), callback); cbp.getCallback(events.execCmd()).apply(null, null); + ss.registerCallback(events.shellCmd(), callback); + cbp.getCallback(events.shellCmd()).apply(null, null); assertThat(callback.count).isEqualTo(Events.MAX_EVENTS); } @@ -293,6 +295,8 @@ public void testThrowableBlocking() { cbp.getCallback(events.loginFailure()).apply(null, null, null); ss.registerCallback(events.execCmd(), throwback); cbp.getCallback(events.execCmd()).apply(null, null); + ss.registerCallback(events.shellCmd(), throwback); + cbp.getCallback(events.shellCmd()).apply(null, null); assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS); } diff --git a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java index 3c5920e2543..eff78d098eb 100644 --- a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java +++ b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java @@ -25,7 +25,7 @@ public interface Capabilities { long CAPABILITY_ASM_RASP_SQLI = 1 << 21; long CAPABILITY_ASM_RASP_LFI = 1 << 22; long CAPABILITY_ASM_RASP_SSRF = 1 << 23; - long CAPABILITY_ASM_RASP_CMDI = 1 << 24; + long CAPABILITY_ASM_RASP_SHI = 1 << 24; long CAPABILITY_ASM_RASP_XXE = 1 << 25; long CAPABILITY_ASM_RASP_RCE = 1 << 26; long CAPABILITY_ASM_RASP_NOSQLI = 1 << 27; @@ -37,4 +37,5 @@ public interface Capabilities { long CAPABILITY_ASM_SESSION_FINGERPRINT = 1L << 33; long CAPABILITY_ASM_NETWORK_FINGERPRINT = 1L << 34; long CAPABILITY_ASM_HEADER_FINGERPRINT = 1L << 35; + long CAPABILITY_ASM_RASP_CMDI = 1 << 37; } From 996ab775575f84c953ca04da27262f73fabd8d5b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Dec 2024 15:57:50 +0100 Subject: [PATCH 14/20] Add metrics to cmdi and shi with rule_variant tag --- .../datadog/appsec/gateway/GatewayBridge.java | 2 +- .../datadog/trace/api/telemetry/RuleType.java | 58 +++++++++++++++---- .../api/telemetry/WafMetricCollector.java | 33 ++++++++++- .../telemetry/WafMetricCollectorTest.groovy | 40 ++++++++++++- 4 files changed, 117 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index fb4aec27e0b..bd0bfd4b0c0 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -304,7 +304,7 @@ private Flow onShellCmd(RequestContext ctx_, String command) { DataBundle bundle = new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.SHELL_CMD, command).build(); try { - GatewayContext gwCtx = new GatewayContext(true, RuleType.COMMAND_INJECTION); + GatewayContext gwCtx = new GatewayContext(true, RuleType.SHELL_INJECTION); return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); } catch (ExpiredSubscriberInfoException e) { shellCmdSubInfo = null; diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java index 6b9217138f4..e4e8276c0eb 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/RuleType.java @@ -1,24 +1,62 @@ package datadog.trace.api.telemetry; +import javax.annotation.Nullable; + public enum RuleType { - LFI("lfi"), - SQL_INJECTION("sql_injection"), - SSRF("ssrf"), - COMMAND_INJECTION("command_injection"); + LFI(Type.LFI), + SQL_INJECTION(Type.SQL_INJECTION), + SSRF(Type.SSRF), + SHELL_INJECTION(Type.COMMAND_INJECTION, Variant.SHELL), + COMMAND_INJECTION(Type.COMMAND_INJECTION, Variant.EXEC); - public final String name; + public final Type type; + @Nullable public final Variant variant; private static final int numValues = RuleType.values().length; - RuleType(String name) { - this.name = name; + RuleType(Type type) { + this(type, null); + } + + RuleType(Type type, @Nullable Variant variant) { + this.type = type; + this.variant = variant; } public static int getNumValues() { return numValues; } - @Override - public String toString() { - return name; + public enum Type { + LFI("lfi"), + SQL_INJECTION("sql_injection"), + SSRF("ssrf"), + COMMAND_INJECTION("command_injection"); + + public final String name; + + Type(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } + } + + public enum Variant { + SHELL("shell"), + EXEC("exec"); + + public final String name; + + Variant(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } } } diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java index 3f093106d28..23acf58653f 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java @@ -269,19 +269,46 @@ public WafRequestsRawMetric( public static class RaspRuleEval extends WafMetric { public RaspRuleEval(final long counter, final RuleType ruleType, final String wafVersion) { - super("rasp.rule.eval", counter, "rule_type:" + ruleType, "waf_version:" + wafVersion); + super( + "rasp.rule.eval", + counter, + ruleType.variant != null + ? new String[] { + "rule_type:" + ruleType.type, + "rule_variant:" + ruleType.variant, + "waf_version:" + wafVersion + } + : new String[] {"rule_type:" + ruleType.type, "waf_version:" + wafVersion}); } } public static class RaspRuleMatch extends WafMetric { public RaspRuleMatch(final long counter, final RuleType ruleType, final String wafVersion) { - super("rasp.rule.match", counter, "rule_type:" + ruleType, "waf_version:" + wafVersion); + super( + "rasp.rule.match", + counter, + ruleType.variant != null + ? new String[] { + "rule_type:" + ruleType.type, + "rule_variant:" + ruleType.variant, + "waf_version:" + wafVersion + } + : new String[] {"rule_type:" + ruleType.type, "waf_version:" + wafVersion}); } } public static class RaspTimeout extends WafMetric { public RaspTimeout(final long counter, final RuleType ruleType, final String wafVersion) { - super("rasp.timeout", counter, "rule_type:" + ruleType, "waf_version:" + wafVersion); + super( + "rasp.timeout", + counter, + ruleType.variant != null + ? new String[] { + "rule_type:" + ruleType.type, + "rule_variant:" + ruleType.variant, + "waf_version:" + wafVersion + } + : new String[] {"rule_type:" + ruleType.type, "waf_version:" + wafVersion}); } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy index 7baafe02e8c..bb3c1d2e4a2 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy @@ -29,8 +29,6 @@ class WafMetricCollectorTest extends DDSpecification { WafMetricCollector.get().raspRuleEval(RuleType.SQL_INJECTION) WafMetricCollector.get().raspTimeout(RuleType.SQL_INJECTION) - - WafMetricCollector.get().prepareMetrics() then: @@ -202,4 +200,42 @@ class WafMetricCollectorTest extends DDSpecification { metric.value == 1 metric.tags == [] } + + def "test Rasp #ruleType metrics"() { + when: + WafMetricCollector.get().wafInit('waf_ver1', 'rules.1') + WafMetricCollector.get().raspRuleEval(ruleType) + WafMetricCollector.get().raspRuleEval(ruleType) + WafMetricCollector.get().raspRuleMatch(ruleType) + WafMetricCollector.get().raspRuleEval(ruleType) + WafMetricCollector.get().raspTimeout(ruleType) + WafMetricCollector.get().prepareMetrics() + + then: + def metrics = WafMetricCollector.get().drain() + + def raspRuleEval = (WafMetricCollector.RaspRuleEval)metrics[1] + raspRuleEval.type == 'count' + raspRuleEval.value == 3 + raspRuleEval.namespace == 'appsec' + raspRuleEval.metricName == 'rasp.rule.eval' + raspRuleEval.tags.toSet() == ['rule_type:command_injection', 'rule_variant:'+ruleType.variant, 'waf_version:waf_ver1'].toSet() + + def raspRuleMatch = (WafMetricCollector.RaspRuleMatch)metrics[2] + raspRuleMatch.type == 'count' + raspRuleMatch.value == 1 + raspRuleMatch.namespace == 'appsec' + raspRuleMatch.metricName == 'rasp.rule.match' + raspRuleMatch.tags.toSet() == ['rule_type:command_injection', 'rule_variant:'+ruleType.variant, 'waf_version:waf_ver1'].toSet() + + def raspTimeout = (WafMetricCollector.RaspTimeout)metrics[3] + raspTimeout.type == 'count' + raspTimeout.value == 1 + raspTimeout.namespace == 'appsec' + raspTimeout.metricName == 'rasp.timeout' + raspTimeout.tags.toSet() == ['rule_type:command_injection', 'rule_variant:'+ruleType.variant, 'waf_version:waf_ver1'].toSet() + + where: + ruleType << [RuleType.COMMAND_INJECTION, RuleType.SHELL_INJECTION] + } } From cf855f79292a00bd5dc1aa0b051358d27000ac3f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 13 Dec 2024 16:12:54 +0100 Subject: [PATCH 15/20] Add another test --- .../RuntimeInstrumentationForkedTest.groovy | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy index 86fabb46177..b909c93e340 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy @@ -63,7 +63,6 @@ class RuntimeInstrumentationForkedTest extends AgentTestRunner{ // ignore } - then: cmdiExpected * callbackProvider.getCallback(EVENTS.execCmd()) >> listener shiExpected * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener @@ -78,4 +77,37 @@ class RuntimeInstrumentationForkedTest extends AgentTestRunner{ [['/bin/evilCommand'] as String[], ['test'] as String[]] | 1 | 0 [['/bin/evilCommand'] as String[], ['test'] as String[], new File('')] | 1 | 0 } + + void 'test shiCheck reset'() { + + setup: + final callbackProvider = Mock(CallbackProvider) + final listener = Mock(BiFunction) + final flow = Mock(Flow) + tracer.getCallbackProvider(RequestContextSlot.APPSEC) >> callbackProvider + + when: + try { + Runtime.getRuntime().exec('$(cat /etc/passwd 1>&2 ; echo .)') + }catch (Exception e){ + // ignore + } + + then: + 0 * callbackProvider.getCallback(EVENTS.execCmd()) >> listener + 1 * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener + 1 * listener.apply(reqCtx, _) >> flow + + when: + try { + Runtime.getRuntime().exec(['/bin/evilCommand'] as String[]) + }catch (Exception e){ + // ignore + } + + then: + 1 * callbackProvider.getCallback(EVENTS.execCmd()) >> listener + 0 * callbackProvider.getCallback(EVENTS.shellCmd()) >> listener + 1 * listener.apply(reqCtx, _) >> flow + } } From 92f9021ce989d1be6cf009c1b98460da958401cb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Sat, 14 Dec 2024 11:51:34 +0100 Subject: [PATCH 16/20] fix cmdi capability --- .../src/main/java/datadog/remoteconfig/Capabilities.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java index eff78d098eb..5aea9ca50bd 100644 --- a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java +++ b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java @@ -37,5 +37,5 @@ public interface Capabilities { long CAPABILITY_ASM_SESSION_FINGERPRINT = 1L << 33; long CAPABILITY_ASM_NETWORK_FINGERPRINT = 1L << 34; long CAPABILITY_ASM_HEADER_FINGERPRINT = 1L << 35; - long CAPABILITY_ASM_RASP_CMDI = 1 << 37; + long CAPABILITY_ASM_RASP_CMDI = 1L << 37; } From 7846c746283b639f2ec0cfe184c4b13082084527 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 16 Dec 2024 11:57:24 +0100 Subject: [PATCH 17/20] change cmdi payloads --- .../java/lang/RuntimeInstrumentationForkedTest.groovy | 8 ++++---- .../datadog/smoketest/appsec/SpringBootSmokeTest.groovy | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy index b909c93e340..acfc16a5b9b 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/RuntimeInstrumentationForkedTest.groovy @@ -73,9 +73,9 @@ class RuntimeInstrumentationForkedTest extends AgentTestRunner{ ['$(cat /etc/passwd 1>&2 ; echo .)'] | 0 | 1 ['$(cat /etc/passwd 1>&2 ; echo .)', ['test'] as String[]] | 0 | 1 ['$(cat /etc/passwd 1>&2 ; echo .)', ['test'] as String[], new File('')] | 0 | 1 - [['/bin/evilCommand'] as String[]] | 1 | 0 - [['/bin/evilCommand'] as String[], ['test'] as String[]] | 1 | 0 - [['/bin/evilCommand'] as String[], ['test'] as String[], new File('')] | 1 | 0 + [['/bin/../usr/bin/reboot', '-f'] as String[]] | 1 | 0 + [['/bin/../usr/bin/reboot', '-f'] as String[], ['test'] as String[]] | 1 | 0 + [['/bin/../usr/bin/reboot', '-f'] as String[], ['test'] as String[], new File('')] | 1 | 0 } void 'test shiCheck reset'() { @@ -100,7 +100,7 @@ class RuntimeInstrumentationForkedTest extends AgentTestRunner{ when: try { - Runtime.getRuntime().exec(['/bin/evilCommand'] as String[]) + Runtime.getRuntime().exec(['/bin/../usr/bin/reboot', '-f'] as String[]) }catch (Exception e){ // ignore } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index d44da4c93fc..3159892c73b 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -597,10 +597,10 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { where: endpoint | cmd | params - 'arrayCmd' | ['/bin/evilCommand'] | null + 'arrayCmd' | ['/bin/../usr/bin/reboot', '-f'] | null 'arrayCmdWithParams' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] 'arrayCmdWithParamsAndFile' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] - 'processBuilder' | ['/bin/evilCommand'] | null + 'processBuilder' | ['/bin/../usr/bin/reboot', '-f'] | null } void 'rasp blocks on SHI'() { From 431cba17d7eca01f6a4f9c54f0742b90a9593e28 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 16 Dec 2024 12:10:48 +0100 Subject: [PATCH 18/20] format test --- .../datadog/smoketest/appsec/SpringBootSmokeTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy index 3159892c73b..3830e31113e 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy @@ -597,10 +597,10 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest { where: endpoint | cmd | params - 'arrayCmd' | ['/bin/../usr/bin/reboot', '-f'] | null + 'arrayCmd' | ['/bin/../usr/bin/reboot', '-f'] | null 'arrayCmdWithParams' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] 'arrayCmdWithParamsAndFile' | ['/bin/../usr/bin/reboot', '-f'] | ['param'] - 'processBuilder' | ['/bin/../usr/bin/reboot', '-f'] | null + 'processBuilder' | ['/bin/../usr/bin/reboot', '-f'] | null } void 'rasp blocks on SHI'() { From 650378e87ef21a526540fd456f14853ea43edaaa Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 16 Dec 2024 12:14:16 +0100 Subject: [PATCH 19/20] fix comment --- .../api/java/lang/ProcessImplInstrumentationHelpers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java index 7289de437a6..cdac9cdc89a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/java/lang/ProcessImplInstrumentationHelpers.java @@ -263,7 +263,7 @@ public static void resetCheckShi() { } /* - Check if there is a cmd injection attempt to block it + Check if there is a chell injection attempt to block it */ public static void shiRaspCheck(@Nonnull final String cmd) { if (!Config.get().isAppSecRaspEnabled()) { From 5615a0aa655badcf05c9b47a42e56397ad8edbbe Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 16 Dec 2024 13:46:20 +0100 Subject: [PATCH 20/20] change Runtime instrumentation to Appsec --- .../trace/instrumentation/java/lang/RuntimeInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java index 6b91b6129c9..00824a1c003 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/RuntimeInstrumentation.java @@ -10,7 +10,7 @@ import java.io.File; @AutoService(InstrumenterModule.class) -public class RuntimeInstrumentation extends InstrumenterModule.Tracing +public class RuntimeInstrumentation extends InstrumenterModule.AppSec implements Instrumenter.ForSingleType, Instrumenter.ForBootstrap { public RuntimeInstrumentation() {