diff --git a/dd-java-agent/agent-iast/README.MD b/dd-java-agent/agent-iast/README.MD new file mode 100644 index 00000000000..9afa59d1afa --- /dev/null +++ b/dd-java-agent/agent-iast/README.MD @@ -0,0 +1,70 @@ +# agent-iast + +## Benchmark results + +### String + +#### concat() + +| Benchmark | Mode | Cnt | Score | Error | Units | +|-------------------------------------|------|-------|---------|---------|-------| +| StringConcatBenchmark.baseline | ss | 15000 | 50.977 | ± 1.561 | ns/op | +| StringConcatBenchmark.iastDisabled | ss | 15000 | 52.963 | ± 0.748 | ns/op | +| StringConcatBenchmark.notTainted | ss | 15000 | 77.463 | ± 0.990 | ns/op | +| StringConcatBenchmark.stringTainted | ss | 15000 | 132.859 | ± 3.565 | ns/op | +| StringConcatBenchmark.paramTainted | ss | 15000 | 156.825 | ± 4.530 | ns/op | +| StringConcatBenchmark.bothTainted | ss | 15000 | 145.427 | ± 3.138 | ns/op | + +### String Builder + +#### constructor() + +| Benchmark | Mode | Cnt | Score | Error | Units | +|-----------------------------------------|------|-------|---------|---------|-------| +| StringBuilderInitBenchmark.baseline | ss | 15000 | 43.278 | ± 0.666 | ns/op | +| StringBuilderInitBenchmark.iastDisabled | ss | 15000 | 45.373 | ± 2.391 | ns/op | +| StringBuilderInitBenchmark.notTainted | ss | 15000 | 66.833 | ± 1.292 | ns/op | +| StringBuilderInitBenchmark.tainted | ss | 15000 | 100.316 | ± 2.767 | ns/op | + +#### append() + +| Benchmark | Mode | Cnt | Score | Error | Units | +|---------------------------------------------------|------|-------|---------|---------|-------| +| StringBuilderAppendBenchmark.baseline | ss | 15000 | 50.261 | ± 2.212 | ns/op | +| StringBuilderAppendBenchmark.iastDisabled | ss | 15000 | 52.746 | ± 0.567 | ns/op | +| StringBuilderAppendBenchmark.notTainted | ss | 15000 | 90.821 | ± 2.245 | ns/op | +| StringBuilderAppendBenchmark.stringBuilderTainted | ss | 15000 | 79.958 | ± 2.289 | ns/op | +| StringBuilderAppendBenchmark.paramTainted | ss | 15000 | 116.093 | ± 3.961 | ns/op | +| StringBuilderAppendBenchmark.bothTainted | ss | 15000 | 107.229 | ± 4.275 | ns/op | + +#### toString() + +| Benchmark | Mode | Cnt | Score | Error | Units | +|---------------------------------------------|------|-------|--------|---------|-------| +| StringBuilderToStringBenchmark.baseline | ss | 15000 | 29.817 | ± 2.493 | ns/op | +| StringBuilderToStringBenchmark.iastDisabled | ss | 15000 | 30.570 | ± 1.794 | ns/op | +| StringBuilderToStringBenchmark.notTainted | ss | 15000 | 57.370 | ± 1.333 | ns/op | +| StringBuilderToStringBenchmark.tainted | ss | 15000 | 92.077 | ± 1.775 | ns/op | + +### batch append operations + +| Benchmark | (stringCount) | (taintedPct) | Mode | Cnt | Score | Error | Units | +|-------------------------------------------|---------------|--------------|------|-------|-------|-------|-------| +| StringBuilderBatchBenchmark.baseline | 10 | 0 | ss | 15000 | 0.348 | 0.009 | us/op | +| StringBuilderBatchBenchmark.baseline | 10 | 50 | ss | 15000 | 0.317 | 0.009 | us/op | +| StringBuilderBatchBenchmark.baseline | 10 | 100 | ss | 15000 | 0.355 | 0.010 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 10 | 0 | ss | 15000 | 0.355 | 0.009 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 10 | 50 | ss | 15000 | 0.344 | 0.008 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 10 | 100 | ss | 15000 | 0.370 | 0.013 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 10 | 0 | ss | 15000 | 0.551 | 0.014 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 10 | 50 | ss | 15000 | 0.794 | 0.014 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 10 | 100 | ss | 15000 | 0.900 | 0.014 | us/op | +| StringBuilderBatchBenchmark.baseline | 100 | 0 | ss | 15000 | 2.508 | 0.025 | us/op | +| StringBuilderBatchBenchmark.baseline | 100 | 50 | ss | 15000 | 2.419 | 0.019 | us/op | +| StringBuilderBatchBenchmark.baseline | 100 | 100 | ss | 15000 | 2.499 | 0.026 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 100 | 0 | ss | 15000 | 2.499 | 0.023 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 100 | 50 | ss | 15000 | 2.608 | 0.180 | us/op | +| StringBuilderBatchBenchmark.iastDisabled | 100 | 100 | ss | 15000 | 2.596 | 0.201 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 100 | 0 | ss | 15000 | 3.426 | 0.028 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 100 | 50 | ss | 15000 | 6.676 | 0.275 | us/op | +| StringBuilderBatchBenchmark.iastEnabled | 100 | 100 | ss | 15000 | 7.539 | 0.164 | us/op | diff --git a/dd-java-agent/agent-iast/build.gradle b/dd-java-agent/agent-iast/build.gradle index a2fe9691b4b..72c3bf9b934 100644 --- a/dd-java-agent/agent-iast/build.gradle +++ b/dd-java-agent/agent-iast/build.gradle @@ -17,6 +17,10 @@ dependencies { testImplementation project(':utils:test-utils') jmh project(':utils:test-utils') + jmh project(':dd-trace-core') + jmh project(':dd-java-agent:agent-builder') + jmh project(':dd-java-agent:instrumentation:iast-instrumenter') + jmh project(':dd-java-agent:instrumentation:java-lang') } sourceCompatibility = JavaVersion.VERSION_1_8 diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/AbstractBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/AbstractBenchmark.java new file mode 100644 index 00000000000..2190dbc1b30 --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/AbstractBenchmark.java @@ -0,0 +1,138 @@ +package com.datadog.iast.propagation; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.IastSystem; +import com.datadog.iast.model.Range; +import com.datadog.iast.model.Source; +import datadog.trace.api.Config; +import datadog.trace.api.gateway.InstrumentationGateway; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.ScopeSource; +import datadog.trace.bootstrap.instrumentation.api.TagContext; +import datadog.trace.common.writer.Writer; +import datadog.trace.core.CoreTracer; +import datadog.trace.core.DDSpan; +import java.util.List; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@State(Scope.Thread) +@OutputTimeUnit(NANOSECONDS) +@BenchmarkMode(Mode.SingleShotTime) +@Warmup(iterations = 50_000) +@Measurement(iterations = 5_000) +@Fork(value = 3) +public abstract class AbstractBenchmark { + + private static final Logger LOG = LoggerFactory.getLogger(AbstractBenchmark.class); + + private AgentSpan span; + private AgentScope scope; + protected C context; + + @Setup(Level.Trial) + public void setup() { + final InstrumentationGateway gateway = new InstrumentationGateway(); + IastSystem.start(gateway.getSubscriptionService(RequestContextSlot.IAST)); + final CoreTracer tracer = + CoreTracer.builder().instrumentationGateway(gateway).writer(new NoOpWriter()).build(); + AgentTracer.forceRegister(tracer); + } + + @Setup(Level.Iteration) + public void start() { + context = initializeContext(); + final TagContext tagContext = new TagContext(); + if (Config.get().isIastEnabled()) { + tagContext.withRequestContextDataIast(context.getIastContext()); + } + final AgentTracer.TracerAPI tracer = AgentTracer.get(); + span = tracer.startSpan("benchmark", tagContext, true); + scope = tracer.activateSpan(span, ScopeSource.INSTRUMENTATION); + } + + @TearDown(Level.Iteration) + public void stop() { + scope.close(); + span.finish(); + } + + protected abstract C initializeContext(); + + protected E tainted(final IastRequestContext context, final E value, final Range... ranges) { + final E result = notTainted(value); + context.getTaintedObjects().taint(result, ranges); + return result; + } + + @SuppressWarnings({"StringOperationCanBeSimplified", "unchecked"}) + protected E notTainted(final E value) { + final E result; + if (value instanceof String) { + result = (E) new String((String) value); + } else { + result = value; + } + computeHash(result); // compute it before to ensure all tests compare the same + return result; + } + + protected Source source() { + return new Source((byte) 0, "key", "value"); + } + + private static long computeHash(final Object value) { + final long hash = System.identityHashCode(value); + LOG.trace("{} hash: {}", value, hash); + return hash; + } + + protected abstract static class BenchmarkContext { + + private final IastRequestContext iastContext; + + protected BenchmarkContext(final IastRequestContext iasContext) { + this.iastContext = iasContext; + } + + public IastRequestContext getIastContext() { + return iastContext; + } + } + + private static class NoOpWriter implements Writer { + + @Override + public void write(final List trace) {} + + @Override + public void start() {} + + @Override + public boolean flush() { + return false; + } + + @Override + public void close() {} + + @Override + public void incrementDropCounts(final int spanCount) {} + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderAppendBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderAppendBenchmark.java new file mode 100644 index 00000000000..8e8c360ed03 --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderAppendBenchmark.java @@ -0,0 +1,98 @@ +package com.datadog.iast.propagation; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Range; +import datadog.trace.api.iast.InstrumentationBridge; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; + +public class StringBuilderAppendBenchmark + extends AbstractBenchmark { + + @Override + protected Context initializeContext() { + final IastRequestContext context = new IastRequestContext(); + final String notTainted = notTainted("I am not a tainted string"); + final String tainted = tainted(context, "I am a tainted string", new Range(5, 6, source())); + final StringBuilder notTaintedBuilder = + notTainted(new StringBuilder("I am not a tainted string builder")); + final StringBuilder taintedBuilder = + tainted( + context, new StringBuilder("I am a tainted string builder"), new Range(5, 6, source())); + return new Context(context, notTainted, tainted, notTaintedBuilder, taintedBuilder); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public StringBuilder baseline() { + return context.notTaintedBuilder.append(context.notTainted); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public StringBuilder iastDisabled() { + final String param = context.notTainted; + final StringBuilder self = context.notTaintedBuilder.append(param); + InstrumentationBridge.onStringBuilderAppend(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder notTainted() { + final String param = context.notTainted; + final StringBuilder self = context.notTaintedBuilder.append(param); + InstrumentationBridge.onStringBuilderAppend(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder paramTainted() { + final String param = context.tainted; + final StringBuilder self = context.notTaintedBuilder.append(param); + InstrumentationBridge.onStringBuilderAppend(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder stringBuilderTainted() { + final String param = context.notTainted; + final StringBuilder self = context.taintedBuilder.append(param); + InstrumentationBridge.onStringBuilderAppend(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder bothTainted() { + final String param = context.tainted; + final StringBuilder self = context.taintedBuilder.append(param); + InstrumentationBridge.onStringBuilderAppend(self, param); + return self; + } + + protected static class Context extends AbstractBenchmark.BenchmarkContext { + + private final String notTainted; + private final String tainted; + + private final StringBuilder notTaintedBuilder; + + private final StringBuilder taintedBuilder; + + protected Context( + final IastRequestContext context, + final String notTainted, + final String tainted, + final StringBuilder notTaintedBuilder, + final StringBuilder taintedBuilder) { + super(context); + this.tainted = tainted; + this.notTainted = notTainted; + this.notTaintedBuilder = notTaintedBuilder; + this.taintedBuilder = taintedBuilder; + } + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderBatchBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderBatchBenchmark.java new file mode 100644 index 00000000000..8b70ab75ef3 --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderBatchBenchmark.java @@ -0,0 +1,91 @@ +package com.datadog.iast.propagation; + +import static java.util.concurrent.TimeUnit.MICROSECONDS; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Range; +import datadog.trace.api.iast.InstrumentationBridge; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.UUID; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; + +@OutputTimeUnit(MICROSECONDS) +public class StringBuilderBatchBenchmark + extends AbstractBenchmark { + + @Param({"10", "100"}) + public int stringCount; + + @Param({"0", "50", "100"}) + public int taintedPct; + + @Override + protected StringBuilderBatchBenchmark.Context initializeContext() { + final IastRequestContext context = new IastRequestContext(); + final List values = new ArrayList<>(stringCount); + final double limit = taintedPct / 100D; + for (int i = 0; i < stringCount; i++) { + double current = i / (double) stringCount; + final String value; + if (current < limit) { + value = tainted(context, UUID.randomUUID().toString(), new Range(3, 6, source())); + } else { + value = notTainted(UUID.randomUUID().toString()); + } + values.add(value); + } + Collections.shuffle(values); + return new StringBuilderBatchBenchmark.Context(context, values); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String baseline() { + final StringBuilder builder = new StringBuilder(); + for (final String string : context.strings) { + builder.append(string); + } + return builder.toString(); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String iastDisabled() { + final StringBuilder builder = new StringBuilder(); + for (final String string : context.strings) { + builder.append(string); + InstrumentationBridge.onStringBuilderAppend(builder, string); + } + final String result = builder.toString(); + InstrumentationBridge.onStringBuilderToString(builder, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String iastEnabled() { + final StringBuilder builder = new StringBuilder(); + for (final String string : context.strings) { + builder.append(string); + InstrumentationBridge.onStringBuilderAppend(builder, string); + } + final String result = builder.toString(); + InstrumentationBridge.onStringBuilderToString(builder, result); + return result; + } + + protected static class Context extends AbstractBenchmark.BenchmarkContext { + + private final List strings; + + protected Context(final IastRequestContext context, final List strings) { + super(context); + this.strings = strings; + } + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderInitBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderInitBenchmark.java new file mode 100644 index 00000000000..7d2ca846fcc --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderInitBenchmark.java @@ -0,0 +1,65 @@ +package com.datadog.iast.propagation; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Range; +import datadog.trace.api.iast.InstrumentationBridge; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; + +public class StringBuilderInitBenchmark + extends AbstractBenchmark { + + @Override + protected Context initializeContext() { + final IastRequestContext context = new IastRequestContext(); + final String notTainted = notTainted("I am not a tainted string"); + final String tainted = tainted(context, "I am a tainted string", new Range(3, 6, source())); + return new Context(context, notTainted, tainted); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public StringBuilder baseline() { + return new StringBuilder(context.notTainted); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public StringBuilder iastDisabled() { + final String param = context.notTainted; + final StringBuilder self = new StringBuilder(param); + InstrumentationBridge.onStringBuilderInit(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder notTainted() { + final String param = context.notTainted; + final StringBuilder self = new StringBuilder(param); + InstrumentationBridge.onStringBuilderInit(self, param); + return self; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public StringBuilder tainted() { + final String param = context.tainted; + final StringBuilder self = new StringBuilder(param); + InstrumentationBridge.onStringBuilderInit(self, param); + return self; + } + + protected static class Context extends AbstractBenchmark.BenchmarkContext { + + private final String notTainted; + private final String tainted; + + protected Context( + final IastRequestContext context, final String notTainted, final String tainted) { + super(context); + this.tainted = tainted; + this.notTainted = notTainted; + } + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderToStringBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderToStringBenchmark.java new file mode 100644 index 00000000000..799edd0527e --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringBuilderToStringBenchmark.java @@ -0,0 +1,71 @@ +package com.datadog.iast.propagation; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Range; +import datadog.trace.api.iast.InstrumentationBridge; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; + +public class StringBuilderToStringBenchmark + extends AbstractBenchmark { + + @Override + protected Context initializeContext() { + final IastRequestContext context = new IastRequestContext(); + final StringBuilder notTaintedBuilder = + notTainted(new StringBuilder("I am not a tainted string builder")); + final StringBuilder taintedBuilder = + tainted( + context, new StringBuilder("I am a tainted string builder"), new Range(5, 7, source())); + return new Context(context, notTaintedBuilder, taintedBuilder); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String baseline() { + return context.notTaintedBuilder.toString(); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String iastDisabled() { + final StringBuilder self = context.notTaintedBuilder; + final String result = self.toString(); + InstrumentationBridge.onStringBuilderToString(self, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String notTainted() { + final StringBuilder self = context.notTaintedBuilder; + final String result = self.toString(); + InstrumentationBridge.onStringBuilderToString(self, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String tainted() { + final StringBuilder self = context.taintedBuilder; + final String result = self.toString(); + InstrumentationBridge.onStringBuilderToString(self, result); + return result; + } + + protected static class Context extends AbstractBenchmark.BenchmarkContext { + + private final StringBuilder notTaintedBuilder; + + private final StringBuilder taintedBuilder; + + protected Context( + final IastRequestContext context, + final StringBuilder notTaintedBuilder, + final StringBuilder taintedBuilder) { + super(context); + this.notTaintedBuilder = notTaintedBuilder; + this.taintedBuilder = taintedBuilder; + } + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringConcatBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringConcatBenchmark.java new file mode 100644 index 00000000000..8ae7a99390b --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/propagation/StringConcatBenchmark.java @@ -0,0 +1,89 @@ +package com.datadog.iast.propagation; + +import com.datadog.iast.IastRequestContext; +import com.datadog.iast.model.Range; +import datadog.trace.api.iast.InstrumentationBridge; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; + +public class StringConcatBenchmark extends AbstractBenchmark { + + @Override + protected StringConcatBenchmark.Context initializeContext() { + final IastRequestContext context = new IastRequestContext(); + final String notTainted = notTainted("I am not a tainted string"); + final String tainted = tainted(context, "I am a tainted string", new Range(3, 5, source())); + return new StringConcatBenchmark.Context(context, notTainted, tainted); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String baseline() { + final String self = context.notTainted; + final String param = context.notTainted; + return self.concat(param); + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=false"}) + public String iastDisabled() { + final String self = context.notTainted; + final String param = context.notTainted; + final String result = self.concat(param); + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String notTainted() { + final String self = context.notTainted; + final String param = context.notTainted; + final String result = self.concat(param); + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String paramTainted() { + final String self = context.notTainted; + final String param = context.tainted; + final String result = self.concat(param); + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String stringTainted() { + final String self = context.tainted; + final String param = context.notTainted; + final String result = self.concat(param); + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } + + @Benchmark + @Fork(jvmArgsAppend = {"-Ddd.iast.enabled=true"}) + public String bothTainted() { + final String self = context.tainted; + final String param = context.tainted; + final String result = self.concat(param); + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } + + protected static class Context extends AbstractBenchmark.BenchmarkContext { + + private final String notTainted; + private final String tainted; + + protected Context( + final IastRequestContext context, final String notTainted, final String tainted) { + super(context); + this.notTainted = notTainted; + this.tainted = tainted; + } + } +} diff --git a/dd-java-agent/agent-iast/src/jmh/resources/logback.xml b/dd-java-agent/agent-iast/src/jmh/resources/logback.xml new file mode 100644 index 00000000000..070671efcfd --- /dev/null +++ b/dd-java-agent/agent-iast/src/jmh/resources/logback.xml @@ -0,0 +1,14 @@ + + + + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n + + + + + + + diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastModuleImpl.java index 26025f19739..594920875f8 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastModuleImpl.java @@ -19,10 +19,13 @@ import datadog.trace.util.stacktrace.StackWalker; import datadog.trace.util.stacktrace.StackWalkerFactory; import java.util.Locale; +import javax.annotation.Nonnull; import javax.annotation.Nullable; public final class IastModuleImpl implements IastModule { + private static final int NULL_STR_LENGTH = "null".length(); + private final Config config; private final Reporter reporter; private final OverheadController overheadController; @@ -35,7 +38,7 @@ public IastModuleImpl( this.overheadController = overheadController; } - public void onCipherAlgorithm(String algorithm) { + public void onCipherAlgorithm(@Nullable final String algorithm) { if (algorithm == null) { return; } @@ -58,7 +61,7 @@ public void onCipherAlgorithm(String algorithm) { reporter.report(span, vulnerability); } - public void onHashingAlgorithm(String algorithm) { + public void onHashingAlgorithm(@Nullable final String algorithm) { if (algorithm == null) { return; } @@ -82,7 +85,7 @@ public void onHashingAlgorithm(String algorithm) { } @Override - public void onParameterName(final @Nullable String paramName) { + public void onParameterName(@Nullable final String paramName) { if (paramName == null || paramName.isEmpty()) { return; } @@ -97,7 +100,7 @@ public void onParameterName(final @Nullable String paramName) { @Override public void onParameterValue( - final @Nullable String paramName, final @Nullable String paramValue) { + @Nullable final String paramName, @Nullable final String paramValue) { if (paramValue == null || paramValue.isEmpty()) { return; } @@ -111,12 +114,12 @@ public void onParameterValue( } @Override - public void onConcat( - @Nullable String left, @Nullable String right, final @Nullable String result) { + public void onStringConcat( + @Nonnull final String left, @Nullable final String right, @Nonnull final String result) { if (!canBeTainted(result)) { return; } - if (!canBeTainted(left) && !canBeTainted(right)) { + if (!canBeTainted(left) && !canBeTaintedNullSafe(right)) { return; } final IastRequestContext ctx = IastRequestContext.get(); @@ -124,40 +127,115 @@ public void onConcat( return; } final TaintedObjects taintedObjects = ctx.getTaintedObjects(); - final Range[] rangesLeft; - if (left == null) { - rangesLeft = Ranges.EMPTY; - left = "null"; + final TaintedObject taintedLeft = getTainted(taintedObjects, left); + final TaintedObject taintedRight = getTainted(taintedObjects, right); + if (taintedLeft == null && taintedRight == null) { + return; + } + final Range[] ranges; + if (taintedRight == null) { + ranges = taintedLeft.getRanges(); + } else if (taintedLeft == null) { + ranges = new Range[taintedRight.getRanges().length]; + Ranges.copyShift(taintedRight.getRanges(), ranges, 0, left.length()); } else { - final TaintedObject to = taintedObjects.get(left); - rangesLeft = (to == null) ? Ranges.EMPTY : to.getRanges(); - } - final Range[] rangesRight; - if (left == right) { - rangesRight = rangesLeft; - } else if (right == null) { - rangesRight = Ranges.EMPTY; - right = "null"; + ranges = mergeRanges(left.length(), taintedLeft.getRanges(), taintedRight.getRanges()); + } + taintedObjects.taint(result, ranges); + } + + @Override + public void onStringBuilderInit( + @Nonnull final StringBuilder builder, @Nullable final CharSequence param) { + if (!canBeTaintedNullSafe(param)) { + return; + } + final IastRequestContext ctx = IastRequestContext.get(); + if (ctx == null) { + return; + } + final TaintedObjects taintedObjects = ctx.getTaintedObjects(); + final TaintedObject paramTainted = taintedObjects.get(param); + if (paramTainted == null) { + return; + } + taintedObjects.taint(builder, paramTainted.getRanges()); + } + + @Override + public void onStringBuilderAppend( + @Nonnull final StringBuilder builder, @Nullable final CharSequence param) { + if (!canBeTainted(builder) || !canBeTaintedNullSafe(param)) { + return; + } + final IastRequestContext ctx = IastRequestContext.get(); + if (ctx == null) { + return; + } + final TaintedObjects taintedObjects = ctx.getTaintedObjects(); + final TaintedObject paramTainted = taintedObjects.get(param); + if (paramTainted == null) { + return; + } + final TaintedObject builderTainted = taintedObjects.get(builder); + final int shift = builder.length() - param.length(); + if (builderTainted == null) { + final Range[] paramRanges = paramTainted.getRanges(); + final Range[] ranges = new Range[paramRanges.length]; + Ranges.copyShift(paramRanges, ranges, 0, shift); + taintedObjects.taint(builder, ranges); } else { - final TaintedObject to = taintedObjects.get(right); - rangesRight = (to == null) ? Ranges.EMPTY : to.getRanges(); + final Range[] builderRanges = builderTainted.getRanges(); + final Range[] paramRanges = paramTainted.getRanges(); + final Range[] ranges = mergeRanges(shift, builderRanges, paramRanges); + builderTainted.setRanges(ranges); } - final int nRanges = rangesLeft.length + rangesRight.length; - if (nRanges == 0) { + } + + @Override + public void onStringBuilderToString( + @Nonnull final StringBuilder builder, @Nonnull final String result) { + if (!canBeTainted(builder) || !canBeTainted(result)) { + return; + } + final IastRequestContext ctx = IastRequestContext.get(); + if (ctx == null) { return; } + final TaintedObjects taintedObjects = ctx.getTaintedObjects(); + final TaintedObject to = taintedObjects.get(builder); + if (to == null) { + return; + } + taintedObjects.taint(result, to.getRanges()); + } + + private static Range[] getRanges(final TaintedObject taintedObject) { + return taintedObject == null ? Ranges.EMPTY : taintedObject.getRanges(); + } + + private static TaintedObject getTainted(final TaintedObjects to, final Object value) { + return value == null ? null : to.get(value); + } + + private static boolean canBeTainted(@Nonnull final CharSequence s) { + return s.length() > 0; + } + + private static boolean canBeTaintedNullSafe(@Nullable final CharSequence s) { + return s != null && canBeTainted(s); + } + + private static Range[] mergeRanges( + final int offset, @Nonnull final Range[] rangesLeft, @Nonnull final Range[] rangesRight) { + final int nRanges = rangesLeft.length + rangesRight.length; final Range[] ranges = new Range[nRanges]; if (rangesLeft.length > 0) { System.arraycopy(rangesLeft, 0, ranges, 0, rangesLeft.length); } if (rangesRight.length > 0) { - final int offset = left.length(); Ranges.copyShift(rangesRight, ranges, rangesLeft.length, offset); } - taintedObjects.taint(result, ranges); - } - - private static boolean canBeTainted(final String s) { - return s != null && !s.isEmpty(); + return ranges; } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java index ed782e5d39c..919aa8877be 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java @@ -17,8 +17,12 @@ public static Range[] forString(final @Nonnull String obj, final @Nonnull Source public static void copyShift( final @Nonnull Range[] src, final @Nonnull Range[] dst, final int dstPos, final int shift) { - for (int iSrc = 0, iDst = dstPos; iSrc < src.length; iSrc++, iDst++) { - dst[iDst] = src[iSrc].shift(shift); + if (shift == 0) { + System.arraycopy(src, 0, dst, dstPos, src.length); + } else { + for (int iSrc = 0, iDst = dstPos; iSrc < src.length; iSrc++, iDst++) { + dst[iDst] = src[iSrc].shift(shift); + } } } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java index 5bda73c013c..5e268a43ff0 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java @@ -11,7 +11,7 @@ public class TaintedObject extends WeakReference { final int positiveHashCode; TaintedObject next; - private final Range[] ranges; + private Range[] ranges; public TaintedObject( final @Nonnull Object obj, @@ -30,4 +30,8 @@ public TaintedObject( public Range[] getRanges() { return ranges; } + + public void setRanges(@Nonnull final Range[] ranges) { + this.ranges = ranges; + } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderAppendTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderAppendTest.groovy new file mode 100644 index 00000000000..2c204bfc36a --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderAppendTest.groovy @@ -0,0 +1,110 @@ +package com.datadog.iast + +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import spock.lang.Shared + +import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.fromTaintFormat +import static com.datadog.iast.taint.TaintUtils.getStringFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.taintFormat + +class IastModuleImplOnStringBuilderAppendTest extends IastModuleImplTestBase { + + @Shared + private List objectHolder = [] + + def setup() { + objectHolder.clear() + } + + void 'onStringBuilderAppend null or empty (#builder, #param)'(final StringBuilder builder, final String param) { + given: + final result = builder?.append(param) + + when: + module.onStringBuilderAppend(result, param) + + then: + 0 * _ + + where: + builder | param + sb('') | null + sb('') | '' + } + + void 'onStringBuilderAppend without span (#builder, #param)'(final StringBuilder builder, final String param, final int mockCalls) { + given: + final result = builder?.append(param) + + when: + module.onStringBuilderAppend(result, param) + + then: + mockCalls * tracer.activeSpan() >> null + 0 * _ + + where: + builder | param | mockCalls + sb('1') | null | 0 + sb('3') | '4' | 1 + } + + void 'onStringBuilderAppend (#builder, #param)'(StringBuilder builder, String param, final int mockCalls, final String expected) { + given: + final span = Mock(AgentSpan) + tracer.activeSpan() >> span + final reqCtx = Mock(RequestContext) + span.getRequestContext() >> reqCtx + final ctx = new IastRequestContext() + reqCtx.getData(RequestContextSlot.IAST) >> ctx + + and: + final taintedObjects = ctx.getTaintedObjects() + builder = addFromTaintFormat(taintedObjects, builder) + objectHolder.add(builder) + param = addFromTaintFormat(taintedObjects, param) + objectHolder.add(param) + + and: + final result = getStringFromTaintFormat(expected) + objectHolder.add(result) + final shouldBeTainted = fromTaintFormat(expected) != null + + when: + builder = builder?.append(param) + module.onStringBuilderAppend(builder, param) + + then: + mockCalls * tracer.activeSpan() >> span + mockCalls * span.getRequestContext() >> reqCtx + mockCalls * reqCtx.getData(RequestContextSlot.IAST) >> ctx + 0 * _ + def to = ctx.getTaintedObjects().get(builder) + if (shouldBeTainted) { + assert to != null + assert to.get() as String == result + assert taintFormat(to.get() as String, to.getRanges()) == expected + } else { + assert to == null + } + + where: + builder | param | mockCalls | expected + sb('123') | null | 0 | '123null' + sb('123') | '456' | 1 | '123456' + sb('==>123<==') | null | 0 | '==>123<==null' + sb('==>123<==') | '456' | 1 | '==>123<==456' + sb('123') | '==>456<==' | 1 | '123==>456<==' + sb('==>123<==') | '==>456<==' | 1 | '==>123<====>456<==' + sb('1==>234<==5==>678<==9') | 'a==>bcd<==e' | 1 | '1==>234<==5==>678<==9a==>bcd<==e' + sb('1==>234<==5==>678<==9') | 'a==>bcd<==e==>fgh<==i' | 1 | '1==>234<==5==>678<==9a==>bcd<==e==>fgh<==i' + } + + private static StringBuilder sb(final String string) { + return new StringBuilder(string) + } +} + diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderInitTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderInitTest.groovy new file mode 100644 index 00000000000..338da2fdf8a --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderInitTest.groovy @@ -0,0 +1,110 @@ +package com.datadog.iast + +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import spock.lang.Shared + +import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.fromTaintFormat +import static com.datadog.iast.taint.TaintUtils.getStringFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.taintFormat + +class IastModuleImplOnStringBuilderInitTest extends IastModuleImplTestBase { + + @Shared + private List objectHolder = [] + + def setup() { + objectHolder.clear() + } + + void 'onStringBuilderInit null or empty (#builder, #param)'(final StringBuilder builder, final String param) { + given: + final result = builder?.append(param) + + when: + module.onStringBuilderInit(result, param) + + then: + 0 * _ + + where: + builder | param + sb('') | null + sb('') | '' + } + + void 'onStringBuilderInit without span (#builder, #param)'(final StringBuilder builder, final String param, final int mockCalls) { + given: + final result = builder?.append(param) + + when: + module.onStringBuilderInit(result, param) + + then: + mockCalls * tracer.activeSpan() >> null + 0 * _ + + where: + builder | param | mockCalls + sb() | null | 0 + sb() | '4' | 1 + } + + void 'onStringBuilderInit (#builder, #param)'(StringBuilder builder, String param, final int mockCalls, final String expected) { + given: + final span = Mock(AgentSpan) + tracer.activeSpan() >> span + final reqCtx = Mock(RequestContext) + span.getRequestContext() >> reqCtx + final ctx = new IastRequestContext() + reqCtx.getData(RequestContextSlot.IAST) >> ctx + + and: + final taintedObjects = ctx.getTaintedObjects() + builder = addFromTaintFormat(taintedObjects, builder) + objectHolder.add(builder) + param = addFromTaintFormat(taintedObjects, param) + objectHolder.add(param) + + and: + final result = getStringFromTaintFormat(expected) + objectHolder.add(result) + final shouldBeTainted = fromTaintFormat(expected) != null + + when: + builder = builder?.append(param) + module.onStringBuilderInit(builder, param) + + then: + mockCalls * tracer.activeSpan() >> span + mockCalls * span.getRequestContext() >> reqCtx + mockCalls * reqCtx.getData(RequestContextSlot.IAST) >> ctx + 0 * _ + def to = ctx.getTaintedObjects().get(builder) + if (shouldBeTainted) { + assert to != null + assert to.get() as String == result + assert taintFormat(to.get() as String, to.getRanges()) == expected + } else { + assert to == null + } + + where: + builder | param | mockCalls | expected + sb() | null | 0 | 'null' + sb() | '123' | 1 | '123' + sb() | '==>123<==' | 1 | '==>123<==' + sb() | 'a==>bcd<==e==>fgh<==i' | 1 | 'a==>bcd<==e==>fgh<==i' + } + + private static StringBuilder sb() { + return sb("") + } + + private static StringBuilder sb(final String string) { + return new StringBuilder(string) + } +} + diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderToStringTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderToStringTest.groovy new file mode 100644 index 00000000000..537f7f4477b --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringBuilderToStringTest.groovy @@ -0,0 +1,84 @@ +package com.datadog.iast + +import datadog.trace.api.gateway.RequestContext +import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import spock.lang.Shared + +import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.fromTaintFormat +import static com.datadog.iast.taint.TaintUtils.getStringFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.taintFormat + +class IastModuleImplOnStringBuilderToStringTest extends IastModuleImplTestBase { + + @Shared + private List objectHolder = [] + + def setup() { + objectHolder.clear() + } + + void 'onStringBuilderToString without span'() { + given: + final builder = sb('1') + final result = builder.toString() + + when: + module.onStringBuilderToString(builder, result) + + then: + 1 * tracer.activeSpan() >> null + 0 * _ + } + + void 'onStringBuilderToString (#builder)'(StringBuilder builder, final String expected) { + given: + final span = Mock(AgentSpan) + tracer.activeSpan() >> span + final reqCtx = Mock(RequestContext) + span.getRequestContext() >> reqCtx + final ctx = new IastRequestContext() + reqCtx.getData(RequestContextSlot.IAST) >> ctx + + and: + final taintedObjects = ctx.getTaintedObjects() + builder = addFromTaintFormat(taintedObjects, builder) + objectHolder.add(builder) + + and: + final result = getStringFromTaintFormat(expected) + objectHolder.add(result) + final shouldBeTainted = fromTaintFormat(expected) != null + + when: + final toString = builder?.toString() + module.onStringBuilderToString(builder, toString) + + then: + 1 * tracer.activeSpan() >> span + 1 * span.getRequestContext() >> reqCtx + 1 * reqCtx.getData(RequestContextSlot.IAST) >> ctx + 0 * _ + def to = ctx.getTaintedObjects().get(builder) + if (shouldBeTainted) { + assert to != null + assert to.get() as String == result + assert taintFormat(to.get() as String, to.getRanges()) == expected + } else { + assert to == null + } + + where: + builder | expected + sb('123') | '123' + sb('==>123<==') | '==>123<==' + sb('==>123<==456') | '==>123<==456' + } + + private static StringBuilder sb(final String string) { + return new StringBuilder(string) + } +} + + diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnConcatTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringConcatTest.groovy similarity index 67% rename from dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnConcatTest.groovy rename to dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringConcatTest.groovy index 9b674e192cf..c99a9cb2295 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnConcatTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastModuleImplOnStringConcatTest.groovy @@ -3,19 +3,28 @@ package com.datadog.iast import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import spock.lang.Shared -import static com.datadog.iast.taint.TaintUtils.* +import static com.datadog.iast.taint.TaintUtils.addFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.fromTaintFormat +import static com.datadog.iast.taint.TaintUtils.getStringFromTaintFormat +import static com.datadog.iast.taint.TaintUtils.taintFormat -class IastModuleImplOnConcatTest extends IastModuleImplTestBase { +class IastModuleImplOnStringConcatTest extends IastModuleImplTestBase { - def objectHolder = new ArrayList() + @Shared + private List objectHolder = [] - void 'onConcat null or empty'() { + def setup() { + objectHolder.clear() + } + + void 'onStringConcat null or empty (#left, #right)'() { given: final result = left + right when: - module.onConcat(left, right, result) + module.onStringConcat(left, right, result) then: 0 * _ @@ -23,16 +32,15 @@ class IastModuleImplOnConcatTest extends IastModuleImplTestBase { where: left | right "" | null - null | "" "" | "" } - void 'onConcat without span'() { + void 'onStringConcat without span (#left, #right)'() { given: final result = left + right when: - module.onConcat(left, right, result) + module.onStringConcat(left, right, result) then: 1 * tracer.activeSpan() >> null @@ -41,11 +49,10 @@ class IastModuleImplOnConcatTest extends IastModuleImplTestBase { where: left | right "1" | null - null | "2" "3" | "4" } - void 'onConcat'() { + void 'onStringConcat (#left, #right)'() { given: final span = Mock(AgentSpan) tracer.activeSpan() >> span @@ -63,11 +70,11 @@ class IastModuleImplOnConcatTest extends IastModuleImplTestBase { and: final result = getStringFromTaintFormat(expected) - objectHolder.add(result) - final shouldBeTainted = fromTaintFormat(expected) != null + objectHolder.add(expected) + final shouldBeTainted = fromTaintFormat(result) != null when: - module.onConcat(left, right, result) + module.onStringConcat(left, right, expected) then: 1 * tracer.activeSpan() >> span @@ -86,10 +93,8 @@ class IastModuleImplOnConcatTest extends IastModuleImplTestBase { where: left | right | expected "123" | null | "123null" - null | "123" | "null123" "123" | "456" | "123456" "==>123<==" | null | "==>123<==null" - null | "==>123<==" | "null==>123<==" "==>123<==" | "456" | "==>123<==456" "123" | "==>456<==" | "123==>456<==" "==>123<==" | "==>456<==" | "==>123<====>456<==" diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy index 9aef3c9723d..3297a6ab2d4 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy @@ -76,6 +76,17 @@ class TaintUtils { return resultString } + static StringBuilder addFromTaintFormat(final TaintedObjects tos, final StringBuilder sb) { + final String s = sb.toString() + final ranges = fromTaintFormat(s) + if (ranges == null || ranges.length == 0) { + return sb + } + final result = new StringBuilder(getStringFromTaintFormat(s)) + tos.taint(result, ranges) + return result + } + static Range toRange(List lst) { toRange(lst.get(0), lst.get(1)) } diff --git a/dd-java-agent/instrumentation/java-lang/build.gradle b/dd-java-agent/instrumentation/java-lang/build.gradle new file mode 100644 index 00000000000..76b14944379 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/build.gradle @@ -0,0 +1,25 @@ +muzzle { + pass { + coreJdk() + } +} + +apply from: "$rootDir/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' +apply plugin: 'call-site-instrumentation' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + testRuntimeClasspath project(':dd-java-agent:instrumentation:iast-instrumenter') +} + +tasks.compileTestJava.configure { + setJavaVersion(it, 8) +} + diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java new file mode 100644 index 00000000000..23e674799a9 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java @@ -0,0 +1,72 @@ +package datadog.trace.instrumentation.java.lang; + +import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.iast.IastAdvice; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.util.stacktrace.StackUtils; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +@CallSite(spi = IastAdvice.class) +public class StringBuilderCallSite { + + @CallSite.AfterArray( + value = { + @CallSite.After("void java.lang.StringBuilder.(java.lang.String)"), + @CallSite.After("void java.lang.StringBuilder.(java.lang.CharSequence)") + }) + @Nonnull + public static StringBuilder afterInit( + @CallSite.This @Nonnull final StringBuilder self, + @CallSite.Argument @Nullable final CharSequence param) { + InstrumentationBridge.onStringBuilderInit(self, param); + return self; + } + + @CallSite.AfterArray( + value = { + @CallSite.After("java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.String)"), + @CallSite.After( + "java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.CharSequence)") + }) + @Nonnull + public static StringBuilder afterAppend( + @CallSite.This @Nonnull final StringBuilder self, + @CallSite.Argument @Nullable final CharSequence param, + @CallSite.Return @Nonnull final StringBuilder result) { + InstrumentationBridge.onStringBuilderAppend(self, param); + return result; + } + + @CallSite.Around("java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.Object)") + @Nonnull + @SuppressFBWarnings( + "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE") // we do check for null on self + // parameter + public static StringBuilder aroundAppend( + @CallSite.This @Nullable final StringBuilder self, + @CallSite.Argument @Nullable final Object param) + throws Throwable { + try { + if (self == null) { + throw new NullPointerException(); + } + final String paramStr = String.valueOf(param); + final StringBuilder result = self.append(paramStr); + InstrumentationBridge.onStringBuilderAppend(result, paramStr); + return result; + } catch (final Throwable e) { + throw StackUtils.filterDatadog(e); + } + } + + @CallSite.After("java.lang.String java.lang.StringBuilder.toString()") + @Nonnull + public static String afterToString( + @CallSite.This @Nonnull final StringBuilder self, + @CallSite.Return @Nonnull final String result) { + InstrumentationBridge.onStringBuilderToString(self, result); + return result; + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java new file mode 100644 index 00000000000..49921e09023 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java @@ -0,0 +1,21 @@ +package datadog.trace.instrumentation.java.lang; + +import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.iast.IastAdvice; +import datadog.trace.api.iast.InstrumentationBridge; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +@CallSite(spi = IastAdvice.class) +public class StringCallSite { + + @CallSite.After("java.lang.String java.lang.String.concat(java.lang.String)") + @Nonnull + public static String afterConcat( + @CallSite.This @Nonnull final String self, + @CallSite.Argument @Nullable final String param, + @CallSite.Return @Nonnull final String result) { + InstrumentationBridge.onStringConcat(self, param, result); + return result; + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy new file mode 100644 index 00000000000..ccc99d6cf89 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy @@ -0,0 +1,154 @@ +package datadog.trace.instrumentation.java.lang + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.IastModule +import datadog.trace.api.iast.InstrumentationBridge +import foo.bar.TestStringBuilderSuite + +class StringBuilderCallSiteTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig("dd.iast.enabled", "true") + } + + def 'test string builder new call site'(final CharSequence param, final String expected) { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + + when: + final result = param.class == String ? + TestStringBuilderSuite.init((String) param) : + TestStringBuilderSuite.init(param) + + then: + result.toString() == expected + if (param.class == String) { + 1 * iastModule.onStringBuilderInit(_ as StringBuilder, (String) param) + } else { + 1 * iastModule.onStringBuilderInit(_ as StringBuilder, param) + } + 0 * _ + + where: + param | expected + new StringBuffer('Hello World!') | 'Hello World!' + 'Hello World!' | 'Hello World!' + } + + def 'test string builder append call site'(final CharSequence param, final String expected) { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + final target = new StringBuilder('Hello ') + + when: + if (param.class == String) { + TestStringBuilderSuite.append(target, (String) param) + } else { + TestStringBuilderSuite.append(target, param) + } + + then: + target.toString() == expected + if (param.class == String) { + 1 * iastModule.onStringBuilderAppend(target, (String) param) + } else { + 1 * iastModule.onStringBuilderAppend(target, param) + } + 0 * _ + + where: + param | expected + new StringBuffer('World!') | 'Hello World!' + 'World!' | 'Hello World!' + } + + def 'test string builder toString call site'() { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + final target = new StringBuilder('Hello World!') + + when: + final result = TestStringBuilderSuite.toString(target) + + then: + result == 'Hello World!' + 1 * iastModule.onStringBuilderToString(target, _ as String) + 0 * _ + } + + def 'test string builder call site in plus operations (JDK8)'() { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + + when: + final result = TestStringBuilderSuite.plus('Hello ', 'World!') + + then: + result == 'Hello World!' + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, 'Hello ') + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, 'World!') + 1 * iastModule.onStringBuilderToString(_ as StringBuilder, 'Hello World!') + 0 * _ + } + + def 'test string builder call site in plus operations with multiple objects (JDK8)'() { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + final args = ['Come to my website ', new URL('https://www.datadoghq.com/'), ' today is ', new Date()] as Object[] + final expected = args.join() + + when: + final result = TestStringBuilderSuite.plus(args) + + then: + result == expected + + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, '') + 1 * iastModule.onStringBuilderToString(_ as StringBuilder, args[0]) + + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, args[0]) + 1 * iastModule.onStringBuilderToString(_ as StringBuilder, args[0..1].join()) + + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, args[0..1].join()) + 1 * iastModule.onStringBuilderToString(_ as StringBuilder, args[0..2].join()) + + 1 * iastModule.onStringBuilderAppend(_ as StringBuilder, args[0..2].join()) + 1 * iastModule.onStringBuilderToString(_ as StringBuilder, expected) + + 0 * _ + } + + def 'test string builder call site in plus operations throwing exceptions (JDK8)'() { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + + when: + TestStringBuilderSuite.plus('Hello', new BrokenToString()) + + then: + 1 * iastModule.onStringBuilderAppend(_, 'Hello') + 0 * _ + final ex = thrown(NuclearException) + ex.stackTrace.find {it.className == StringBuilderCallSite.name } == null + } + + private static class BrokenToString { + @Override + String toString() { + throw new NuclearException('BOOM!!!!') + } + } + + private static class NuclearException extends RuntimeException { + NuclearException(final String message) { + super(message) + } + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringCallSiteTest.groovy new file mode 100644 index 00000000000..d7996cb6280 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringCallSiteTest.groovy @@ -0,0 +1,28 @@ +package datadog.trace.instrumentation.java.lang + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.IastModule +import datadog.trace.api.iast.InstrumentationBridge +import foo.bar.TestStringSuite + +class StringCallSiteTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig("dd.iast.enabled", "true") + } + + def 'test string concat call site'() { + setup: + final iastModule = Mock(IastModule) + InstrumentationBridge.registerIastModule(iastModule) + + when: + final result = TestStringSuite.concat('Hello ', 'World!') + + then: + result == 'Hello World!' + 1 * iastModule.onStringConcat('Hello ', 'World!', 'Hello World!') + 0 * _ + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java new file mode 100644 index 00000000000..f6216c03683 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java @@ -0,0 +1,67 @@ +package foo.bar; + +import java.util.Arrays; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class TestStringBuilderSuite { + + private static final Logger LOGGER = LoggerFactory.getLogger(TestStringBuilderSuite.class); + + public static StringBuilder init(final String param) { + LOGGER.debug("Before new string builder {}", param); + final StringBuilder result = new StringBuilder(param); + LOGGER.debug("After new string builder {}", result); + return result; + } + + public static StringBuilder init(final CharSequence param) { + LOGGER.debug("Before new string builder {}", param); + final StringBuilder result = new StringBuilder(param); + LOGGER.debug("After new string builder {}", result); + return result; + } + + public static void append(final StringBuilder builder, final String param) { + LOGGER.debug("Before string builder append {}", param); + final StringBuilder result = builder.append(param); + LOGGER.debug("After string builder append {}", result); + } + + public static void append(final StringBuilder builder, final CharSequence param) { + LOGGER.debug("Before string builder append {}", param); + final StringBuilder result = builder.append(param); + LOGGER.debug("After string builder append {}", result); + } + + public static String toString(final StringBuilder builder) { + LOGGER.debug("Before string builder toString {}", builder); + final String result = builder.toString(); + LOGGER.debug("After string builder toString {}", result); + return result; + } + + public static String plus(final String left, final String right) { + LOGGER.debug("Before string plus {} {}", left, right); + final String result = left + right; + LOGGER.debug("After string builder toString {}", result); + return result; + } + + public static String plus(final String left, final Object right) { + LOGGER.debug("Before string plus {} {}", left, right); + final String result = left + right; + LOGGER.debug("After string builder toString {}", result); + return result; + } + + public static String plus(final Object... items) { + LOGGER.debug("Before string plus {}", Arrays.toString(items)); + String result = ""; + for (final Object item : items) { + result += item; + } + LOGGER.debug("After string builder toString {}", result); + return result; + } +} diff --git a/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringSuite.java b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringSuite.java new file mode 100644 index 00000000000..c4f882ef375 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringSuite.java @@ -0,0 +1,16 @@ +package foo.bar; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class TestStringSuite { + + private static final Logger LOGGER = LoggerFactory.getLogger(TestStringSuite.class); + + public static String concat(final String left, final String right) { + LOGGER.debug("Before string concat {} {}", left, right); + final String result = left.concat(right); + LOGGER.debug("After string concat {}", result); + return result; + } +} diff --git a/internal-api/internal-api-8/build.gradle b/internal-api/internal-api-8/build.gradle index e4faaf8826d..a6a8e61f12d 100644 --- a/internal-api/internal-api-8/build.gradle +++ b/internal-api/internal-api-8/build.gradle @@ -40,6 +40,7 @@ dependencies { testImplementation deps.commonsMath testImplementation deps.mockito testImplementation deps.slf4j + testImplementation deps.truth testImplementation project(":utils:test-utils") } diff --git a/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/AbstractStackWalker.java b/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/AbstractStackWalker.java index 2e6935b0a28..17c90ae02ab 100644 --- a/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/AbstractStackWalker.java +++ b/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/AbstractStackWalker.java @@ -16,7 +16,7 @@ final Stream doFilterStack(Stream stream) abstract T doGetStack(Function, T> consumer); - private static boolean isNotDatadogTraceStackElement(final StackTraceElement el) { + static boolean isNotDatadogTraceStackElement(final StackTraceElement el) { final String clazz = el.getClassName(); return !clazz.startsWith("datadog.trace.") && !clazz.startsWith("com.datadog.iast."); } diff --git a/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/StackUtils.java b/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/StackUtils.java new file mode 100644 index 00000000000..b64842fa85f --- /dev/null +++ b/internal-api/internal-api-8/src/main/java/datadog/trace/util/stacktrace/StackUtils.java @@ -0,0 +1,56 @@ +package datadog.trace.util.stacktrace; + +import java.util.Arrays; +import java.util.function.Function; +import java.util.function.Predicate; + +public abstract class StackUtils { + + public static E update( + final E exception, final Function filter) { + final StackTraceElement[] stack = exception.getStackTrace(); + exception.setStackTrace(filter.apply(stack)); + return exception; + } + + public static E filter( + final E exception, final Predicate filter) { + return update( + exception, stack -> Arrays.stream(stack).filter(filter).toArray(StackTraceElement[]::new)); + } + + public static E filterFirst( + final E exception, final Predicate filter) { + return filter(exception, new OneTimePredicate<>(filter)); + } + + public static E filterDatadog(final E exception) { + return filter(exception, AbstractStackWalker::isNotDatadogTraceStackElement); + } + + public static E filterFirstDatadog(final E exception) { + return filterFirst(exception, AbstractStackWalker::isNotDatadogTraceStackElement); + } + + private static class OneTimePredicate implements Predicate { + + private final Predicate delegate; + private boolean filtered; + + private OneTimePredicate(final Predicate delegate) { + this.delegate = delegate; + } + + @Override + public boolean test(final T item) { + if (filtered) { + return true; + } + final boolean test = delegate.test(item); + if (!test) { + filtered = true; + } + return test; + } + } +} diff --git a/internal-api/internal-api-8/src/test/java/datadog/trace/util/stacktrace/StackUtilsTest.java b/internal-api/internal-api-8/src/test/java/datadog/trace/util/stacktrace/StackUtilsTest.java new file mode 100644 index 00000000000..00da684d7c6 --- /dev/null +++ b/internal-api/internal-api-8/src/test/java/datadog/trace/util/stacktrace/StackUtilsTest.java @@ -0,0 +1,103 @@ +package datadog.trace.util.stacktrace; + +import static com.google.common.truth.Truth.assertThat; + +import java.util.function.Function; +import org.junit.jupiter.api.Test; + +public class StackUtilsTest { + + @Test + public void test_identity_function() { + final Throwable source = new RuntimeException(); + final StackTraceElement[] expected = source.getStackTrace(); + + final Throwable updated = StackUtils.update(withStack(expected), Function.identity()); + assertThat(updated.getStackTrace()).isEqualTo(expected); + } + + @Test + public void test_filter_all_datadog() { + final StackTraceElement[] stack = + new StackTraceElement[] { + stack().className("org.junit.jupiter.api.Test").build(), + stack().className("datadog.trace.util.stacktrace.StackUtilsTest").build(), + stack().className("java.util.function.Function").build(), + stack().className("datadog.trace.util.stacktrace.StackUtils").build(), + stack().className("com.google.common.truth.Truth").build() + }; + final StackTraceElement[] expected = new StackTraceElement[] {stack[0], stack[2], stack[4]}; + + final Throwable filtered = + StackUtils.filter( + withStack(stack), item -> !item.getClassName().startsWith("datadog.trace")); + assertThat(filtered.getStackTrace()).isEqualTo(expected); + + final Throwable filtered2 = StackUtils.filterDatadog(withStack(stack)); + assertThat(filtered2.getStackTrace()).isEqualTo(expected); + } + + @Test + public void test_filter_first_datadog() { + final StackTraceElement[] stack = + new StackTraceElement[] { + stack().className("org.junit.jupiter.api.Test").build(), + stack().className("datadog.trace.util.stacktrace.StackUtilsTest").build(), + stack().className("java.util.function.Function").build(), + stack().className("datadog.trace.util.stacktrace.StackUtils").build(), + stack().className("com.google.common.truth.Truth").build() + }; + final StackTraceElement[] expected = + new StackTraceElement[] {stack[0], stack[2], stack[3], stack[4]}; + + final Throwable filtered = + StackUtils.filterFirst( + withStack(stack), item -> !item.getClassName().startsWith("datadog.trace")); + assertThat(filtered.getStackTrace()).isEqualTo(expected); + + final Throwable filtered2 = StackUtils.filterFirstDatadog(withStack(stack)); + assertThat(filtered2.getStackTrace()).isEqualTo(expected); + } + + private static Throwable withStack(final StackTraceElement... stack) { + final Exception exception = new RuntimeException(); + exception.setStackTrace(stack); + return exception; + } + + private static StackTraceBuilder stack() { + return new StackTraceBuilder(); + } + + private static class StackTraceBuilder { + + private String className = "mock"; + private String methodName = "mock"; + private String fileName = "mock"; + private int lineNumber; + + public StackTraceBuilder className(final String className) { + this.className = className; + return this; + } + + public StackTraceBuilder methodName(final String methodName) { + this.methodName = methodName; + return this; + } + + public StackTraceBuilder fileName(final String fileName) { + this.fileName = fileName; + return this; + } + + public StackTraceBuilder lineNumber(final int lineNumber) { + this.lineNumber = lineNumber; + return this; + } + + public StackTraceElement build() { + return new StackTraceElement(className, methodName, fileName, lineNumber); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/iast/IastModule.java b/internal-api/src/main/java/datadog/trace/api/iast/IastModule.java index 39db6125ee1..0081d53d5f3 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/IastModule.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/IastModule.java @@ -5,9 +5,9 @@ public interface IastModule { - void onCipherAlgorithm(@Nonnull String algorithm); + void onCipherAlgorithm(@Nullable String algorithm); - void onHashingAlgorithm(@Nonnull String algorithm); + void onHashingAlgorithm(@Nullable String algorithm); /** * An HTTP request parameter name is used. This should be used when it cannot be determined @@ -21,5 +21,11 @@ public interface IastModule { */ void onParameterValue(@Nullable String paramName, @Nullable String paramValue); - void onConcat(@Nullable String left, @Nullable String right, @Nullable String result); + void onStringConcat(@Nonnull String left, @Nullable String right, @Nonnull String result); + + void onStringBuilderInit(@Nonnull StringBuilder builder, @Nullable CharSequence param); + + void onStringBuilderAppend(@Nonnull StringBuilder builder, @Nullable CharSequence param); + + void onStringBuilderToString(@Nonnull StringBuilder builder, @Nonnull String result); } 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 7dd45c92f8a..757beee6a08 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 @@ -1,6 +1,7 @@ package datadog.trace.api.iast; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,6 +73,50 @@ public static void onParameterValue(final String paramName, final String paramVa } } + public static void onStringConcat( + @Nonnull final String self, @Nullable final String param, @Nonnull final String result) { + try { + if (MODULE != null) { + MODULE.onStringConcat(self, param, result); + } + } catch (final Throwable t) { + onUnexpectedException("Callback for onStringConcat threw.", t); + } + } + + public static void onStringBuilderInit( + @Nonnull final StringBuilder self, @Nullable final CharSequence param) { + try { + if (MODULE != null) { + MODULE.onStringBuilderInit(self, param); + } + } catch (final Throwable t) { + onUnexpectedException("Callback for onStringBuilderInit threw.", t); + } + } + + public static void onStringBuilderAppend( + @Nonnull final StringBuilder self, @Nullable final CharSequence param) { + try { + if (MODULE != null) { + MODULE.onStringBuilderAppend(self, param); + } + } catch (final Throwable t) { + onUnexpectedException("Callback for onStringBuilderAppend threw.", t); + } + } + + public static void onStringBuilderToString( + @Nonnull final StringBuilder self, @Nonnull final String result) { + try { + if (MODULE != null) { + MODULE.onStringBuilderToString(self, result); + } + } catch (final Throwable t) { + onUnexpectedException("Callback for onStringBuilderToString threw.", t); + } + } + private static void onUnexpectedException(final String message, final Throwable error) { LOG.warn(message, error); } diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/InstrumentationBridgeTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/InstrumentationBridgeTest.groovy index ecb0b1ea2b0..9359e104a11 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/iast/InstrumentationBridgeTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/InstrumentationBridgeTest.groovy @@ -126,4 +126,151 @@ class InstrumentationBridgeTest extends DDSpecification { 1 * module.onParameterValue(_, _) >> { throw new Error('Boom!!!') } noExceptionThrown() } + + def "bridge calls module when a new string concat is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + + when: + InstrumentationBridge.onStringConcat('Hello ', 'World!', 'Hello World!') + + then: + 1 * module.onStringConcat('Hello ', 'World!', 'Hello World!') + } + + def "bridge calls don't fail with null module when a string concat is detected"() { + setup: + InstrumentationBridge.registerIastModule(null) + + when: + InstrumentationBridge.onStringConcat('Hello ', 'World!', 'Hello World!') + + then: + noExceptionThrown() + } + + def "bridge calls don't leak exceptions when a string concat is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + + when: + InstrumentationBridge.onStringConcat('Hello ', 'World!', 'Hello World!') + + then: + 1 * module.onStringConcat(_, _, _) >> { throw new Error('Boom!!!') } + noExceptionThrown() + } + + def "bridge calls module when a new string builder init is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + final self = new StringBuilder() + + when: + InstrumentationBridge.onStringBuilderInit(self, 'test') + + then: + 1 * module.onStringBuilderInit(self, 'test') + } + + def "bridge calls don't fail with null module when a string builder init is detected"() { + setup: + InstrumentationBridge.registerIastModule(null) + + when: + InstrumentationBridge.onStringBuilderInit(new StringBuilder(), 'test') + + then: + noExceptionThrown() + } + + def "bridge calls don't leak exceptions when a string builder init is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + + when: + InstrumentationBridge.onStringBuilderInit(new StringBuilder(), 'test') + + then: + 1 * module.onStringBuilderInit(_, _) >> { throw new Error('Boom!!!') } + noExceptionThrown() + } + + def "bridge calls module when a new string builder append is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + final self = new StringBuilder() + + when: + InstrumentationBridge.onStringBuilderAppend(self, 'test') + + then: + 1 * module.onStringBuilderAppend(self, 'test') + } + + def "bridge calls don't fail with null module when a string builder append is detected"() { + setup: + InstrumentationBridge.registerIastModule(null) + + when: + InstrumentationBridge.onStringBuilderAppend(new StringBuilder(), 'test') + + then: + noExceptionThrown() + } + + def "bridge calls don't leak exceptions when a string builder append is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + + when: + InstrumentationBridge.onStringBuilderAppend(new StringBuilder(), 'test') + + then: + 1 * module.onStringBuilderAppend(_, _) >> { throw new Error('Boom!!!') } + noExceptionThrown() + } + + def "bridge calls module when a new string builder toString() is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + final self = new StringBuilder() + + when: + InstrumentationBridge.onStringBuilderToString(self, 'test') + + then: + 1 * module.onStringBuilderToString(self, 'test') + } + + def "bridge calls don't fail with null module when a string builder toString is detected"() { + setup: + InstrumentationBridge.registerIastModule(null) + + when: + InstrumentationBridge.onStringBuilderToString(new StringBuilder('test'), 'test') + + then: + noExceptionThrown() + } + + def "bridge calls don't leak exceptions when a string builder toString is detected"() { + setup: + final module = Mock(IastModule) + InstrumentationBridge.registerIastModule(module) + + when: + InstrumentationBridge.onStringBuilderToString(new StringBuilder('test'), 'test') + + then: + 1 * module.onStringBuilderToString(_, _) >> { throw new Error('Boom!!!') } + noExceptionThrown() + } } diff --git a/settings.gradle b/settings.gradle index 03dd878fbed..efdbad9ca66 100644 --- a/settings.gradle +++ b/settings.gradle @@ -189,12 +189,13 @@ include ':dd-java-agent:instrumentation:http-url-connection' include ':dd-java-agent:instrumentation:hystrix-1.4' include ':dd-java-agent:instrumentation:iast-instrumenter' include ':dd-java-agent:instrumentation:ignite-2.0' -include ':dd-java-agent:instrumentation:java-security' include ':dd-java-agent:instrumentation:jakarta-rs-annotations-3' include ':dd-java-agent:instrumentation:java-concurrent' include ':dd-java-agent:instrumentation:java-concurrent:java-completablefuture' include ':dd-java-agent:instrumentation:java-concurrent:lambda-testing' include ':dd-java-agent:instrumentation:java-directbytebuffer' +include ':dd-java-agent:instrumentation:java-lang' +include ':dd-java-agent:instrumentation:java-security' include ':dd-java-agent:instrumentation:jax-rs-annotations-1' include ':dd-java-agent:instrumentation:jax-rs-annotations-2' include ':dd-java-agent:instrumentation:jax-rs-annotations-2:filter-jersey'