diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 5cb79a9856d..91ae0708cff 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -78,6 +78,7 @@ /dd-java-agent/instrumentation/snakeyaml-1.33/ @DataDog/asm-java /dd-java-agent/instrumentation/velocity-1.5/ @DataDog/asm-java /dd-java-agent/instrumentation/freemarker/ @DataDog/asm-java +/dd-java-agent/instrumentation/beanshell-2.0/ @DataDog/asm-java /dd-java-agent/instrumentation/datadog/asm/ @DataDog/asm-java /dd-smoke-tests/apm-tracing-disabled/ @DataDog/asm-java /dd-smoke-tests/appsec/ @DataDog/asm-java diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java index e11712e2030..e1992897a7d 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java @@ -10,6 +10,7 @@ import com.datadog.iast.propagation.StringModuleImpl; import com.datadog.iast.securitycontrol.IastSecurityControlTransformer; import com.datadog.iast.sink.ApplicationModuleImpl; +import com.datadog.iast.sink.CodeInjectionModuleImpl; import com.datadog.iast.sink.CommandInjectionModuleImpl; import com.datadog.iast.sink.EmailInjectionModuleImpl; import com.datadog.iast.sink.HardcodedSecretModuleImpl; @@ -187,7 +188,8 @@ private static Stream iastModules( InsecureAuthProtocolModuleImpl.class, ReflectionInjectionModuleImpl.class, UntrustedDeserializationModuleImpl.class, - EmailInjectionModuleImpl.class); + EmailInjectionModuleImpl.class, + CodeInjectionModuleImpl.class); if (iast != FULLY_ENABLED) { modules = modules.filter(IastSystem::isOptOut); } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java index 0cc7103a548..2c5f3e81684 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java @@ -1,6 +1,7 @@ package com.datadog.iast.model; import static com.datadog.iast.util.CRCUtils.update; +import static datadog.trace.api.iast.VulnerabilityMarks.CODE_INJECTION_MARK; import static datadog.trace.api.iast.VulnerabilityMarks.COMMAND_INJECTION_MARK; import static datadog.trace.api.iast.VulnerabilityMarks.EMAIL_HTML_INJECTION_MARK; import static datadog.trace.api.iast.VulnerabilityMarks.HEADER_INJECTION_MARK; @@ -171,6 +172,9 @@ public interface VulnerabilityType { .excludedSources(Builder.DB_EXCLUDED) .build(); + VulnerabilityType CODE_INJECTION = + type(VulnerabilityTypes.CODE_INJECTION).mark(CODE_INJECTION_MARK).build(); + /* All vulnerability types that have a mark. Should be updated if new vulnerabilityType with mark is added */ VulnerabilityType[] MARKED_VULNERABILITIES = { SQL_INJECTION, @@ -185,7 +189,8 @@ public interface VulnerabilityType { HEADER_INJECTION, REFLECTION_INJECTION, UNTRUSTED_DESERIALIZATION, - EMAIL_HTML_INJECTION + EMAIL_HTML_INJECTION, + CODE_INJECTION }; String name(); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/CodeInjectionModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/CodeInjectionModuleImpl.java new file mode 100644 index 00000000000..770300da0e5 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/CodeInjectionModuleImpl.java @@ -0,0 +1,35 @@ +package com.datadog.iast.sink; + +import static com.datadog.iast.taint.Tainteds.canBeTainted; + +import com.datadog.iast.Dependencies; +import com.datadog.iast.model.VulnerabilityType; +import datadog.trace.api.iast.sink.CodeInjectionModule; +import java.io.InputStreamReader; +import java.io.Reader; +import java.io.StringReader; +import javax.annotation.Nonnull; + +public class CodeInjectionModuleImpl extends SinkModuleBase implements CodeInjectionModule { + + public CodeInjectionModuleImpl(final Dependencies dependencies) { + super(dependencies); + } + + @Override + public void onEval(@Nonnull Reader reader) { + // IAST propagates taint only to StringReader and InputStreamReader. + if (!(reader instanceof StringReader) && !(reader instanceof InputStreamReader)) { + return; + } + checkInjection(VulnerabilityType.CODE_INJECTION, reader); + } + + @Override + public void onEval(@Nonnull String script) { + if (!canBeTainted(script)) { + return; + } + checkInjection(VulnerabilityType.CODE_INJECTION, script); + } +} diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/CodeInjectionModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/CodeInjectionModuleTest.groovy new file mode 100644 index 00000000000..9612280fd79 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/CodeInjectionModuleTest.groovy @@ -0,0 +1,168 @@ +package com.datadog.iast.sink + +import com.datadog.iast.IastModuleImplTestBase +import com.datadog.iast.Reporter +import com.datadog.iast.model.Range +import com.datadog.iast.model.Source +import com.datadog.iast.model.Vulnerability +import com.datadog.iast.model.VulnerabilityType +import com.datadog.iast.taint.Ranges +import datadog.trace.api.iast.SourceTypes +import datadog.trace.api.iast.VulnerabilityMarks +import datadog.trace.api.iast.sink.CodeInjectionModule + +class CodeInjectionModuleTest extends IastModuleImplTestBase { + + private CodeInjectionModule module + + def setup() { + module = new CodeInjectionModuleImpl(dependencies) + } + + @Override + protected Reporter buildReporter() { + return Mock(Reporter) + } + + void 'test null or empty script is ignored'() { + when: 'cast disambiguates the onEval(String) / onEval(Reader) overloads for a null argument' + module.onEval(script as String) + + then: + 0 * _ + + where: + script | _ + null | _ + '' | _ + } + + void 'test code injection detection on string'() { + when: + module.onEval(script) + + then: 'report is not called if no active span' + tracer.activeSpan() >> null + 0 * reporter.report(_, _) + + when: + module.onEval(script) + + then: 'report is not called if the script is not tainted' + tracer.activeSpan() >> span + 0 * reporter.report(_, _) + + when: + taint(script) + module.onEval(script) + + then: 'report is called when the script is tainted' + tracer.activeSpan() >> span + 1 * reporter.report(span, { Vulnerability vul -> vul.type == VulnerabilityType.CODE_INJECTION }) + + where: + script = '2 + 2' + } + + void 'test code injection detection on StringReader'() { + given: + final reader = new StringReader('2 + 2') + + when: + module.onEval(reader) + + then: 'report is not called if the reader is not tainted' + tracer.activeSpan() >> span + 0 * reporter.report(_, _) + + when: + taint(reader) + module.onEval(reader) + + then: 'report is called when the reader is tainted' + tracer.activeSpan() >> span + 1 * reporter.report(span, { Vulnerability vul -> vul.type == VulnerabilityType.CODE_INJECTION }) + } + + void 'test code injection detection on InputStreamReader'() { + given: + final reader = new InputStreamReader(new ByteArrayInputStream('2 + 2'.bytes)) + + when: + module.onEval(reader) + + then: 'report is not called if the reader is not tainted' + tracer.activeSpan() >> span + 0 * reporter.report(_, _) + + when: + taint(reader) + module.onEval(reader) + + then: 'report is called when the reader is tainted' + tracer.activeSpan() >> span + 1 * reporter.report(span, { Vulnerability vul -> vul.type == VulnerabilityType.CODE_INJECTION }) + + cleanup: + reader.close() + } + + void 'test unsupported Reader type is ignored even when tainted'() { + given: 'only StringReader and InputStreamReader are inspected (see CodeInjectionModuleImpl.onEval)' + final reader = new CharArrayReader('2 + 2'.toCharArray()) + taint(reader) + + when: + module.onEval(reader) + + then: + 0 * _ + + cleanup: + reader.close() + } + + void 'if all ranges of the tainted script have the code injection mark it is not reported'() { + given: + final script = '2 + 2' + final Range[] ranges = [ + new Range(0, 1, new Source(SourceTypes.REQUEST_PARAMETER_VALUE, 'name', 'value'), VulnerabilityMarks.CODE_INJECTION_MARK) + ] + ctx.getTaintedObjects().taint(script, ranges) + + when: + module.onEval(script) + + then: + tracer.activeSpan() >> span + 0 * reporter.report(_, _) + } + + void 'if all ranges of the tainted reader have the code injection mark it is not reported'() { + given: + final Range[] ranges = [ + new Range(0, 1, new Source(SourceTypes.REQUEST_PARAMETER_VALUE, 'name', 'value'), VulnerabilityMarks.CODE_INJECTION_MARK) + ] + ctx.getTaintedObjects().taint(reader, ranges) + + when: + module.onEval(reader) + + then: + tracer.activeSpan() >> span + 0 * reporter.report(_, _) + + cleanup: + reader.close() + + where: + reader << [ + new StringReader('2 + 2'), + new InputStreamReader(new ByteArrayInputStream('2 + 2'.bytes)) + ] + } + + private taint(final Object value) { + ctx.getTaintedObjects().taint(value, Ranges.forObject(new Source(SourceTypes.REQUEST_PARAMETER_VALUE, 'name', value.toString()))) + } +} diff --git a/dd-java-agent/instrumentation/beanshell-2.0/build.gradle b/dd-java-agent/instrumentation/beanshell-2.0/build.gradle new file mode 100644 index 00000000000..3b254a1fc0e --- /dev/null +++ b/dd-java-agent/instrumentation/beanshell-2.0/build.gradle @@ -0,0 +1,40 @@ +muzzle { + // BeanShell ships under two coordinates over its lifetime: the original org.beanshell:bsh + // (up to 2.0b5) and the later org.apache-extras.beanshell:bsh (2.0b5, 2.0b6). Cover both. + // 2.0b4 is the supported floor. + pass { + group = 'org.beanshell' + module = 'bsh' + versions = '[2.0b4,]' + assertInverse = true + } + pass { + group = 'org.apache-extras.beanshell' + module = 'bsh' + versions = '[2.0b4,]' + assertInverse = true + } +} + +apply from: "$rootDir/gradle/java.gradle" + +addTestSuiteForDir('latestDepTest', 'test') + +// latestDepTest extends test (see gradle/test-suites.gradle), which drags in the org.beanshell:bsh +// 2.0b4 floor. Drop that coordinate here so its duplicate bsh.* classes don't collide with — or +// shadow on the classpath — the org.apache-extras.beanshell:bsh artifact we actually want to validate. +configurations.latestDepTestImplementation { + exclude group: 'org.beanshell', module: 'bsh' +} + +dependencies { + compileOnly group: 'org.beanshell', name: 'bsh', version: '2.0b4' + + // Unit/instrumentation tests run against the supported floor (2.0b4); latestDepTest re-runs the + // same suite against the newest published version to guard against future regressions. + testImplementation group: 'org.beanshell', name: 'bsh', version: '2.0b4' + + testRuntimeOnly project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter') + + latestDepTestImplementation group: 'org.apache-extras.beanshell', name: 'bsh', version: '+' +} diff --git a/dd-java-agent/instrumentation/beanshell-2.0/src/main/java/datadog/trace/instrumentation/beanshell/BeanShellInstrumentation.java b/dd-java-agent/instrumentation/beanshell-2.0/src/main/java/datadog/trace/instrumentation/beanshell/BeanShellInstrumentation.java new file mode 100644 index 00000000000..57aaad9212b --- /dev/null +++ b/dd-java-agent/instrumentation/beanshell-2.0/src/main/java/datadog/trace/instrumentation/beanshell/BeanShellInstrumentation.java @@ -0,0 +1,160 @@ +package datadog.trace.instrumentation.beanshell; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.muzzle.Reference.EXPECTS_NON_STATIC; +import static datadog.trace.agent.tooling.muzzle.Reference.EXPECTS_PUBLIC; +import static datadog.trace.agent.tooling.muzzle.Reference.EXPECTS_STATIC; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +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.agent.tooling.muzzle.Reference; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Sink; +import datadog.trace.api.iast.VulnerabilityTypes; +import datadog.trace.api.iast.sink.CodeInjectionModule; +import datadog.trace.api.iast.sink.SsrfModule; +import java.io.Reader; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public class BeanShellInstrumentation extends InstrumenterModule.Iast + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public BeanShellInstrumentation() { + super("beanshell"); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] {"bsh.Interpreter", "bsh.Remote"}; + } + + @Override + public Reference[] additionalMuzzleReferences() { + return new Reference[] { + new Reference.Builder("bsh.Interpreter") + .withMethod( + new String[0], + EXPECTS_PUBLIC | EXPECTS_NON_STATIC, + "eval", + "Ljava/lang/Object;", + "Ljava/lang/String;", + "Lbsh/NameSpace;") + .withMethod( + new String[0], + EXPECTS_PUBLIC | EXPECTS_NON_STATIC, + "eval", + "Ljava/lang/Object;", + "Ljava/io/Reader;", + "Lbsh/NameSpace;", + "Ljava/lang/String;") + .build(), + new Reference.Builder("bsh.Remote") + .withMethod( + new String[0], + EXPECTS_PUBLIC | EXPECTS_STATIC, + "eval", + "I", + "Ljava/lang/String;", + "Ljava/lang/String;") + .build(), + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + // bsh.Interpreter.eval(String, NameSpace): entry point that eval(String) also delegates to. + // It builds the Reader internally, but bsh.* is excluded from call-site instrumentation + // (iast_exclusion.trie) so that reader is never tainted; we inspect the String arg directly. + transformer.applyAdvice( + named("eval") + .and(isMethod()) + .and( + takesArguments(2) + .and(takesArgument(0, String.class)) + .and(takesArgument(1, named("bsh.NameSpace")))), + BeanShellInstrumentation.class.getName() + "$StringEvalAdvice"); + // bsh.Interpreter.eval(Reader, NameSpace, String): shared core reached by public eval(Reader). + // Only reports when the caller supplied a tainted Reader; the Reader built internally by + // eval(String, NameSpace) is untainted, so the String path above does not double-report. + transformer.applyAdvice( + named("eval") + .and(isMethod()) + .and( + takesArguments(3) + .and(takesArgument(0, Reader.class)) + .and(takesArgument(1, named("bsh.NameSpace"))) + .and(takesArgument(2, String.class))), + BeanShellInstrumentation.class.getName() + "$EvalAdvice"); + // bsh.Remote.eval(String url, String text) + transformer.applyAdvice( + named("eval").and(isMethod()).and(takesArguments(String.class, String.class)), + BeanShellInstrumentation.class.getName() + "$RemoteEvalAdvice"); + } + + public static class StringEvalAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + @Sink(VulnerabilityTypes.CODE_INJECTION) + public static void onEnter(@Advice.Argument(0) final String statements) { + if (statements == null) { + return; + } + final CodeInjectionModule codeInjectionModule = InstrumentationBridge.CODE_INJECTION; + if (codeInjectionModule == null) { + return; + } + codeInjectionModule.onEval(statements); + } + } + + public static class EvalAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + @Sink(VulnerabilityTypes.CODE_INJECTION) + public static void onEnter(@Advice.Argument(0) final Reader reader) { + if (reader == null) { + return; + } + final CodeInjectionModule codeInjectionModule = InstrumentationBridge.CODE_INJECTION; + if (codeInjectionModule == null) { + return; + } + codeInjectionModule.onEval(reader); + } + } + + public static class RemoteEvalAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + @Sink(VulnerabilityTypes.CODE_INJECTION) + public static void onEnter( + @Advice.Argument(0) final String url, @Advice.Argument(1) final String text) { + // Remote.eval dispatches on the URL scheme: "http:" opens a URL connection and "bsh:" opens a + // raw socket; every other scheme throws before any I/O or script dispatch, so neither the URL + // nor the script reaches a sink. bsh.* is excluded from call-site instrumentation, so neither + // the URL/URLConnection nor the socket SSRF call-site sink fires inside bsh either. + if (url == null || !(url.startsWith("http:") || url.startsWith("bsh:"))) { + return; + } + + final SsrfModule ssrfModule = InstrumentationBridge.SSRF; + if (ssrfModule != null) { + ssrfModule.onURLConnection(url); + } + + if (text == null) { + return; + } + final CodeInjectionModule codeInjectionModule = InstrumentationBridge.CODE_INJECTION; + if (codeInjectionModule == null) { + return; + } + codeInjectionModule.onEval(text); + } + } +} diff --git a/dd-java-agent/instrumentation/beanshell-2.0/src/test/groovy/datadog/trace/instrumentation/beanshell/BeanShellInstrumentationTest.groovy b/dd-java-agent/instrumentation/beanshell-2.0/src/test/groovy/datadog/trace/instrumentation/beanshell/BeanShellInstrumentationTest.groovy new file mode 100644 index 00000000000..ec0a282cbae --- /dev/null +++ b/dd-java-agent/instrumentation/beanshell-2.0/src/test/groovy/datadog/trace/instrumentation/beanshell/BeanShellInstrumentationTest.groovy @@ -0,0 +1,98 @@ +package datadog.trace.instrumentation.beanshell + +import bsh.Interpreter +import bsh.Remote +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.sink.CodeInjectionModule +import datadog.trace.api.iast.sink.SsrfModule + +class BeanShellInstrumentationTest extends InstrumentationSpecification { + + @Override + protected void configurePreAgent() { + injectSysConfig('dd.iast.enabled', 'true') + } + + void 'test Interpreter.eval(String)'() { + given: + final module = Mock(CodeInjectionModule) + InstrumentationBridge.registerIastModule(module) + + when: + new Interpreter().eval('2 + 2;') + + then: + // eval(String) delegates to eval(String, NameSpace) (inspected directly as a String) which then + // builds a StringReader and delegates to the eval(Reader, NameSpace, String) core (inspected as a + // Reader). Both advices fire here; in production the internally built reader is untainted (bsh.* is + // excluded from call-site instrumentation), so only the String check can actually report. + 1 * module.onEval('2 + 2;') + 1 * module.onEval(_ as Reader) + 0 * _ + } + + void 'test Interpreter.eval(Reader)'() { + given: + final module = Mock(CodeInjectionModule) + InstrumentationBridge.registerIastModule(module) + + when: + new Interpreter().eval(new StringReader('2 + 2;')) + + then: + 1 * module.onEval(_ as Reader) + 0 * _ + } + + void 'test Remote.eval routes text to code injection and connecting url schemes to ssrf'() { + given: + final codeInjectionModule = Mock(CodeInjectionModule) + final ssrfModule = Mock(SsrfModule) + InstrumentationBridge.registerIastModule(codeInjectionModule) + InstrumentationBridge.registerIastModule(ssrfModule) + + when: + // Remote.eval is OnMethodEnter-instrumented, so the sinks fire before the body attempts a + // connection; the subsequent connection failure to a dead port is expected and irrelevant here. + try { + Remote.eval(url, '2 + 2;') + } catch (Exception ignored) { + } + + then: + sinkCalls * codeInjectionModule.onEval('2 + 2;') + sinkCalls * ssrfModule.onURLConnection(url) + 0 * _ + + where: + // bsh.Remote dispatches only for the "http:" and "bsh:" schemes; every other scheme throws + // before any I/O or script dispatch, so neither SSRF nor code injection should be reported. + url | sinkCalls + 'bsh://localhost:1/' | 1 + 'http://localhost:1/' | 1 + 'https://localhost:1/' | 0 + 'ftp://localhost:1/' | 0 + } + + void 'test Remote.eval reports ssrf even when the script is null'() { + given: + final codeInjectionModule = Mock(CodeInjectionModule) + final ssrfModule = Mock(SsrfModule) + InstrumentationBridge.registerIastModule(codeInjectionModule) + InstrumentationBridge.registerIastModule(ssrfModule) + + when: + // bsh.Remote opens the socket before it dereferences the script, so a null script must not + // suppress the SSRF report for a connecting URL scheme. + try { + Remote.eval('bsh://localhost:1/', (String) null) + } catch (Exception ignored) { + } + + then: + 1 * ssrfModule.onURLConnection('bsh://localhost:1/') + 0 * codeInjectionModule.onEval(_) + 0 * _ + } +} diff --git a/dd-smoke-tests/iast-util/build.gradle b/dd-smoke-tests/iast-util/build.gradle index 4219698314c..bdd28c8140f 100644 --- a/dd-smoke-tests/iast-util/build.gradle +++ b/dd-smoke-tests/iast-util/build.gradle @@ -25,6 +25,8 @@ dependencies { implementation 'com.sun.mail:jakarta.mail:2.0.1' // text sanitization implementation group: 'org.apache.commons', name: 'commons-text', version: '1.0' + // code injection + compileOnly group: 'org.apache-extras.beanshell', name: 'bsh', version: '2.0b6' testFixturesCompileOnly(libs.bundles.groovy) testFixturesCompileOnly(libs.bundles.spock) diff --git a/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/CodeInjectionController.java b/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/CodeInjectionController.java new file mode 100644 index 00000000000..1dee0d2b61c --- /dev/null +++ b/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/CodeInjectionController.java @@ -0,0 +1,52 @@ +package datadog.smoketest.springboot.controller; + +import bsh.Interpreter; +import bsh.Remote; +import java.io.StringReader; +import javax.servlet.http.HttpServletRequest; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class CodeInjectionController { + + @GetMapping("/code_injection/beanshell") + public String beanshell(final HttpServletRequest request) { + final String param = request.getParameter("param"); + try { + // The CODE_INJECTION sink fires on method entry, before the script is parsed/evaluated, + // so an evaluation failure here does not affect the vulnerability being reported. + new Interpreter().eval(param); + } catch (final Exception e) { + // ignore evaluation errors + } + return "ok"; + } + + @GetMapping("/code_injection/beanshell_reader") + public String beanshellReader(final HttpServletRequest request) { + final String param = request.getParameter("param"); + try { + // Wrapping the tainted parameter in a StringReader propagates the taint to the reader + // (StringReaderCallSite), so eval(Reader) exercises the reader sink path end-to-end. + new Interpreter().eval(new StringReader(param)); + } catch (final Exception e) { + // ignore evaluation errors + } + return "ok"; + } + + @GetMapping("/code_injection/beanshell_remote") + public String beanshellRemote(final HttpServletRequest request) { + final String url = request.getParameter("url"); + final String script = request.getParameter("script"); + try { + // Remote.eval reports CODE_INJECTION on the script and SSRF on the url on method entry, + // before it attempts the (here failing) connection. + Remote.eval(url, script); + } catch (final Exception e) { + // ignore evaluation / connection errors + } + return "ok"; + } +} diff --git a/dd-smoke-tests/springboot/build.gradle b/dd-smoke-tests/springboot/build.gradle index c19aa169476..c390228c8b6 100644 --- a/dd-smoke-tests/springboot/build.gradle +++ b/dd-smoke-tests/springboot/build.gradle @@ -48,6 +48,8 @@ dependencies { implementation 'com.sun.mail:jakarta.mail:2.0.1' // text sanitization implementation group: 'org.apache.commons', name: 'commons-text', version: '1.0' + // code injection + implementation group: 'org.apache-extras.beanshell', name: 'bsh', version: '2.0b6' } tasks.withType(Test).configureEach { diff --git a/dd-smoke-tests/springboot/src/test/groovy/datadog/smoketest/IastSpringBootSmokeTest.groovy b/dd-smoke-tests/springboot/src/test/groovy/datadog/smoketest/IastSpringBootSmokeTest.groovy index ac823de6045..69129527c6b 100644 --- a/dd-smoke-tests/springboot/src/test/groovy/datadog/smoketest/IastSpringBootSmokeTest.groovy +++ b/dd-smoke-tests/springboot/src/test/groovy/datadog/smoketest/IastSpringBootSmokeTest.groovy @@ -27,7 +27,78 @@ class IastSpringBootSmokeTest extends AbstractIastSpringBootTest { hasTainted { it.value == 'jackie' && - it.ranges[0].source.origin == 'http.request.header' + it.ranges[0].source.origin == 'http.request.header' + } + } + + void 'code injection is present (string)'() { + given: + final param = 'test' + final url = "http://localhost:${httpPort}/code_injection/beanshell?param=${param}" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: + hasVulnerability { + vul -> + vul.type == 'CODE_INJECTION' + && vul.location.method == 'beanshell' + && vul.evidence.valueParts.size() == 1 + && vul.evidence.valueParts[0].value == param + && vul.evidence.valueParts[0].source.origin == 'http.request.parameter' + } + } + + void 'code injection is present (reader)'() { + given: + final param = 'test' + final url = "http://localhost:${httpPort}/code_injection/beanshell_reader?param=${param}" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: + // The reader is tainted as an object, so the evidence value is the reader identity rather than + // the script; assert the type, location and source origin instead of the value. + hasVulnerability { + vul -> + vul.type == 'CODE_INJECTION' + && vul.location.method == 'beanshellReader' + && vul.evidence.valueParts[0].source.origin == 'http.request.parameter' + } + } + + void 'code injection is present (remote)'() { + given: + final script = 'test' + final ssrfUrl = 'http://localhost:1/' + final url = "http://localhost:${httpPort}/code_injection/beanshell_remote?" + + "url=${URLEncoder.encode(ssrfUrl, 'UTF-8')}&script=${script}" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: 'the script is reported as a code injection' + hasVulnerability { + vul -> + vul.type == 'CODE_INJECTION' + && vul.location.method == 'beanshellRemote' + && vul.evidence.valueParts.size() == 1 + && vul.evidence.valueParts[0].value == script + && vul.evidence.valueParts[0].source.origin == 'http.request.parameter' + } + + and: 'the url is reported as an ssrf' + hasVulnerability { + vul -> + vul.type == 'SSRF' + && vul.evidence.valueParts.size() == 1 + && vul.evidence.valueParts[0].value == ssrfUrl + && vul.evidence.valueParts[0].source.origin == 'http.request.parameter' } } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java b/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java index 835364fafcf..e5faeb96a4c 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/InstrumentationBridge.java @@ -4,6 +4,7 @@ import datadog.trace.api.iast.propagation.PropagationModule; import datadog.trace.api.iast.propagation.StringModule; import datadog.trace.api.iast.sink.ApplicationModule; +import datadog.trace.api.iast.sink.CodeInjectionModule; import datadog.trace.api.iast.sink.CommandInjectionModule; import datadog.trace.api.iast.sink.EmailInjectionModule; import datadog.trace.api.iast.sink.HardcodedSecretModule; @@ -70,6 +71,7 @@ public abstract class InstrumentationBridge { public static ReflectionInjectionModule REFLECTION_INJECTION; public static UntrustedDeserializationModule UNTRUSTED_DESERIALIZATION; public static EmailInjectionModule EMAIL_INJECTION; + public static CodeInjectionModule CODE_INJECTION; private static final Map, Field> MODULE_MAP = buildModuleMap(); diff --git a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java index ccad045ab18..b1915b4ded6 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java @@ -20,6 +20,7 @@ private VulnerabilityMarks() {} public static final int HEADER_INJECTION_MARK = 1 << 9; public static final int REFLECTION_INJECTION_MARK = 1 << 10; public static final int UNTRUSTED_DESERIALIZATION_MARK = 1 << 11; + public static final int CODE_INJECTION_MARK = 1 << 12; public static final int CUSTOM_SECURITY_CONTROL_MARK = 1 << 13; public static final int EMAIL_HTML_INJECTION_MARK = 1 << 14; @@ -38,6 +39,7 @@ public static int markForAll() { | HEADER_INJECTION_MARK | REFLECTION_INJECTION_MARK | UNTRUSTED_DESERIALIZATION_MARK + | CODE_INJECTION_MARK | CUSTOM_SECURITY_CONTROL_MARK; } @@ -67,10 +69,12 @@ public static int getMarkFromVulnerabitityType(final String vulnerabilityTypeStr return REFLECTION_INJECTION_MARK; case "UNTRUSTED_DESERIALIZATION": return UNTRUSTED_DESERIALIZATION_MARK; - case "CUSTOM_SECURITY_CONTROL": - return CUSTOM_SECURITY_CONTROL_MARK; case "EMAIL_HTML_INJECTION": return EMAIL_HTML_INJECTION_MARK; + case "CODE_INJECTION": + return CODE_INJECTION_MARK; + case "CUSTOM_SECURITY_CONTROL": + return CUSTOM_SECURITY_CONTROL_MARK; default: return NOT_MARKED; } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java index aefb1b663d8..37b9096de6d 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityTypes.java @@ -38,6 +38,7 @@ private VulnerabilityTypes() {} public static final byte DEFAULT_APP_DEPLOYED = 29; public static final byte UNTRUSTED_DESERIALIZATION = 30; public static final byte EMAIL_HTML_INJECTION = 31; + public static final byte CODE_INJECTION = 32; /** * Use for telemetry only, this is a special vulnerability type that is not reported, reported @@ -117,7 +118,8 @@ private VulnerabilityTypes() {} "SESSION_REWRITING", "DEFAULT_APP_DEPLOYED", "UNTRUSTED_DESERIALIZATION", - "EMAIL_HTML_INJECTION" + "EMAIL_HTML_INJECTION", + "CODE_INJECTION" }; public static String toString(final byte vulnerability) { diff --git a/internal-api/src/main/java/datadog/trace/api/iast/sink/CodeInjectionModule.java b/internal-api/src/main/java/datadog/trace/api/iast/sink/CodeInjectionModule.java new file mode 100644 index 00000000000..dc0c8cf9a43 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/sink/CodeInjectionModule.java @@ -0,0 +1,12 @@ +package datadog.trace.api.iast.sink; + +import datadog.trace.api.iast.IastModule; +import java.io.Reader; +import javax.annotation.Nonnull; + +public interface CodeInjectionModule extends IastModule { + + void onEval(@Nonnull Reader reader); + + void onEval(@Nonnull String string); +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy index 90f93548144..df755281e44 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/VulnerabilityTypesTest.groovy @@ -46,5 +46,6 @@ class VulnerabilityTypesTest extends DDSpecification { VulnerabilityTypes.DEFAULT_APP_DEPLOYED | 'DEFAULT_APP_DEPLOYED' VulnerabilityTypes.UNTRUSTED_DESERIALIZATION | 'UNTRUSTED_DESERIALIZATION' VulnerabilityTypes.EMAIL_HTML_INJECTION | 'EMAIL_HTML_INJECTION' + VulnerabilityTypes.CODE_INJECTION | 'CODE_INJECTION' } } diff --git a/metadata/agent-jar-checks.properties b/metadata/agent-jar-checks.properties index 913a83697b1..a7bde3e2525 100644 --- a/metadata/agent-jar-checks.properties +++ b/metadata/agent-jar-checks.properties @@ -28,6 +28,7 @@ expected.integrations = IastInstrumentation,\ axis2,\ axway-api,\ azure-functions,\ + beanshell,\ caffeine,\ cassandra,\ ci-visibility,\ diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 2d3d77ec355..af0caea701f 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -5041,6 +5041,14 @@ "aliases": [] } ], + "DD_TRACE_BEANSHELL_ENABLED": [ + { + "version": "A", + "type": "boolean", + "default": "true", + "aliases": ["DD_TRACE_INTEGRATION_BEANSHELL_ENABLED", "DD_INTEGRATION_BEANSHELL_ENABLED"] + } + ], "DD_TRACE_CAFFEINE_ENABLED": [ { "version": "A", diff --git a/settings.gradle.kts b/settings.gradle.kts index c2ef7c17958..14e988af3fe 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -319,6 +319,7 @@ include( ":dd-java-agent:instrumentation:axis2-1.3", ":dd-java-agent:instrumentation:axway-api-7.5", ":dd-java-agent:instrumentation:azure-functions-1.2.2", + ":dd-java-agent:instrumentation:beanshell-2.0", ":dd-java-agent:instrumentation:caffeine-1.0", ":dd-java-agent:instrumentation:cdi-1.2", ":dd-java-agent:instrumentation:cics-9.1",