Skip to content

Commit

Permalink
Merge branch 'master' into evanchooly/DEBUG-2719
Browse files Browse the repository at this point in the history
  • Loading branch information
evanchooly authored Sep 9, 2024
2 parents 0026f35 + 5699a18 commit 2de3ffb
Show file tree
Hide file tree
Showing 26 changed files with 456 additions and 27 deletions.
9 changes: 1 addition & 8 deletions .circleci/config.continue.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation
debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core"
profiling_modules: &profiling_modules "dd-java-agent/agent-profiling"

default_system_tests_commit: &default_system_tests_commit 39bf65e0d94278c60fdd6ff85ba668cb436467f0
default_system_tests_commit: &default_system_tests_commit c87bd359aad64a29f280fc5c70a879f7c7f4846e

parameters:
nightly:
Expand Down Expand Up @@ -273,13 +273,6 @@ commands:
git fetch origin << parameters.systemTestsCommit >>
git reset --hard FETCH_HEAD

- run:
name: Install python 3.12
command: |
sudo apt-get update
sudo apt-get install -y python3.12-full python3.12-dev python3.12-venv
echo 'export PATH="$HOME/.local/bin:$PATH"' >>"$BASH_ENV"

jobs:
build:
<<: *defaults
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public class DefaultExceptionDebugger implements DebuggerContext.ExceptionDebugg
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultExceptionDebugger.class);
public static final String DD_DEBUG_ERROR_PREFIX = "_dd.debug.error.";
public static final String DD_DEBUG_ERROR_EXCEPTION_ID = DD_DEBUG_ERROR_PREFIX + "exception_id";
public static final String DD_DEBUG_ERROR_EXCEPTION_HASH =
DD_DEBUG_ERROR_PREFIX + "exception_hash";
public static final String ERROR_DEBUG_INFO_CAPTURED = "error.debug_info_captured";
public static final String SNAPSHOT_ID_TAG_FMT = DD_DEBUG_ERROR_PREFIX + "%d.snapshot_id";

Expand Down Expand Up @@ -84,7 +86,7 @@ public void handleException(Throwable t, AgentSpan span) {
LOGGER.debug("Unable to find state for throwable: {}", innerMostException.toString());
return;
}
processSnapshotsAndSetTags(t, span, state, innerMostException);
processSnapshotsAndSetTags(t, span, state, innerMostException, fingerprint);
exceptionProbeManager.updateLastCapture(fingerprint);
} else {
if (exceptionProbeManager.createProbesForException(innerMostException.getStackTrace())) {
Expand All @@ -101,7 +103,11 @@ private void applyExceptionConfiguration(String fingerprint) {
}

private static void processSnapshotsAndSetTags(
Throwable t, AgentSpan span, ThrowableState state, Throwable innerMostException) {
Throwable t,
AgentSpan span,
ThrowableState state,
Throwable innerMostException,
String fingerprint) {
if (span.getTag(DD_DEBUG_ERROR_EXCEPTION_ID) != null) {
LOGGER.debug("Clear previous frame tags");
// already set for this span, clear the frame tags
Expand Down Expand Up @@ -141,6 +147,7 @@ private static void processSnapshotsAndSetTags(
DD_DEBUG_ERROR_EXCEPTION_ID,
state.getExceptionId());
span.setTag(ERROR_DEBUG_INFO_CAPTURED, true);
span.setTag(DD_DEBUG_ERROR_EXCEPTION_HASH, fingerprint);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledIf;
import org.junit.jupiter.api.condition.EnabledForJreRange;
import org.junit.jupiter.api.condition.EnabledOnJre;
import org.junit.jupiter.api.condition.JRE;
Expand Down Expand Up @@ -1725,6 +1726,9 @@ public void evaluateAtExitFalse() throws IOException, URISyntaxException {
}

@Test
@DisabledIf(
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void uncaughtExceptionConditionLocalVar() throws IOException, URISyntaxException {
final String CLASS_NAME = "CapturedSnapshot05";
LogProbe probe =
Expand Down Expand Up @@ -1755,6 +1759,9 @@ public void uncaughtExceptionConditionLocalVar() throws IOException, URISyntaxEx
}

@Test
@DisabledIf(
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void uncaughtExceptionCaptureLocalVars() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "uncaughtException", null);
Expand All @@ -1776,6 +1783,9 @@ public void uncaughtExceptionCaptureLocalVars() throws IOException, URISyntaxExc
}

@Test
@DisabledIf(
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void methodProbeLocalVarsLocalScopes() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "localScopes", "(String)");
Expand All @@ -1789,6 +1799,9 @@ public void methodProbeLocalVarsLocalScopes() throws IOException, URISyntaxExcep
}

@Test
@DisabledIf(
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void methodProbeLocalVarsDeepScopes() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "deepScopes", "(String)");
Expand All @@ -1806,6 +1819,9 @@ public void methodProbeLocalVarsDeepScopes() throws IOException, URISyntaxExcept
}

@Test
@DisabledIf(
value = "datadog.trace.api.Platform#isJ9",
disabledReason = "we cannot get local variable debug info")
public void methodProbeExceptionLocalVars() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31";
LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "caughtException", "(String)");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.datadog.debugger.exception;

import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.REMOTE_CONFIG;
import static com.datadog.debugger.exception.DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_HASH;
import static com.datadog.debugger.exception.DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_ID;
import static com.datadog.debugger.exception.DefaultExceptionDebugger.ERROR_DEBUG_INFO_CAPTURED;
import static com.datadog.debugger.exception.DefaultExceptionDebugger.SNAPSHOT_ID_TAG_FMT;
Expand Down Expand Up @@ -135,6 +136,7 @@ public void instrumentAndCaptureSnapshots() throws Exception {
location.getType() + "." + location.getMethod(), snapshot0.getStack().get(0).getFunction());
MutableSpan span = traceInterceptor.getFirstSpan();
assertEquals(snapshot0.getExceptionId(), span.getTags().get(DD_DEBUG_ERROR_EXCEPTION_ID));
assertEquals(fingerprint, span.getTags().get(DD_DEBUG_ERROR_EXCEPTION_HASH));
assertEquals(Boolean.TRUE, span.getTags().get(ERROR_DEBUG_INFO_CAPTURED));
assertEquals(snapshot0.getId(), span.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0)));
assertEquals(1, probeSampler.getCallCount());
Expand Down
1 change: 1 addition & 0 deletions dd-java-agent/instrumentation/freemarker/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
apply from: "$rootDir/gradle/java.gradle"
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ dependencies {
compileOnly group: 'org.freemarker', name: 'freemarker', version: '2.3.24-incubating'

testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.24-incubating'
testImplementation project(':dd-java-agent:instrumentation:freemarker:freemarker-2.3.9')

testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
muzzle {
fail {
name = 'freemarker-2.3.9'
group = 'org.freemarker'
module = 'freemarker'
versions = '[2.3.24-incubating,]'
assertInverse = true
}
}

apply from: "$rootDir/gradle/java.gradle"

addTestSuiteForDir("version2_3_23Test", "test")

dependencies {
compileOnly group: 'org.freemarker', name: 'freemarker', version: '2.3.9'

testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.9'

testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')

version2_3_23TestImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.23'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package datadog.trace.instrumentation.freemarker9;

import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.iast.InstrumentationBridge;
import datadog.trace.api.iast.Sink;
import datadog.trace.api.iast.VulnerabilityTypes;
import datadog.trace.api.iast.sink.XssModule;
import freemarker.core.DollarVariable2_3_9Helper;
import freemarker.core.Environment;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.matcher.ElementMatcher;

@AutoService(InstrumenterModule.class)
public class DollarVariableInstrumentation extends InstrumenterModule.Iast
implements Instrumenter.ForSingleType {
static final String FREEMARKER_CORE = "freemarker.core";

public DollarVariableInstrumentation() {
super("freemarker");
}

@Override
public String muzzleDirective() {
return "freemarker-2.3.9";
}

static final ElementMatcher.Junction<ClassLoader> NOT_VERSION_PRIOR_2_3_24 =
not(hasClassNamed("freemarker.cache.ByteArrayTemplateLoader"));

@Override
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return NOT_VERSION_PRIOR_2_3_24;
}

@Override
public String instrumentedType() {
return FREEMARKER_CORE + ".DollarVariable";
}

@Override
public String[] helperClassNames() {
return new String[] {FREEMARKER_CORE + ".DollarVariable2_3_9Helper"};
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
named("accept")
.and(isMethod())
.and(takesArgument(0, named(FREEMARKER_CORE + ".Environment"))),
DollarVariableInstrumentation.class.getName() + "$DollarVariableAdvice");
}

public static class DollarVariableAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
@Sink(VulnerabilityTypes.XSS)
public static void onEnter(
@Advice.Argument(0) final Environment environment, @Advice.This final Object self) {
if (environment == null || self == null) {
return;
}
final XssModule xssModule = InstrumentationBridge.XSS;
if (xssModule == null) {
return;
}
String charSec = DollarVariable2_3_9Helper.fetchCharSec(self, environment);
final String templateName = environment.getTemplate().getName();
final int line = DollarVariable2_3_9Helper.fetchBeginLine(self);
xssModule.onXss(charSec, templateName, line);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package freemarker.core;

import freemarker.template.TemplateModelException;
import java.lang.reflect.Field;
import java.lang.reflect.UndeclaredThrowableException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class DollarVariable2_3_9Helper {
private DollarVariable2_3_9Helper() {}

private static final Logger log = LoggerFactory.getLogger(DollarVariable2_3_9Helper.class);

private static final Field ESCAPED_EXPRESSION = prepareEscapedExpression();

private static Field prepareEscapedExpression() {
try {
Field autoEscape = DollarVariable.class.getDeclaredField("escapedExpression");
autoEscape.setAccessible(true);
return autoEscape;
} catch (Throwable e) {
log.debug("Failed to get DollarVariable escapedExpression", e);
return null;
}
}

public static Expression fetchEscapeExpression(Object object) {
if (ESCAPED_EXPRESSION == null || !(object instanceof DollarVariable)) {
return null;
}
try {
return (Expression) ESCAPED_EXPRESSION.get(object);
} catch (IllegalAccessException e) {
throw new UndeclaredThrowableException(e);
}
}

public static String fetchCharSec(Object object, Environment environment) {
if (!(object instanceof DollarVariable)) {
return null;
}
final Expression expression = DollarVariable2_3_9Helper.fetchEscapeExpression(object);
if (expression instanceof BuiltIn) {
return null;
}
try {
return environment.getDataModel().get(expression.toString()).toString();
} catch (TemplateModelException e) {
throw new UndeclaredThrowableException(e);
}
}

public static Integer fetchBeginLine(Object object) {
if (!(object instanceof DollarVariable)) {
return null;
}
return ((DollarVariable) object).beginLine;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package datadog.trace.instrumentation.freemarker9

import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.sink.XssModule
import freemarker.template.Configuration
import freemarker.template.SimpleHash
import freemarker.template.Template
import freemarker.template.TemplateHashModel

class DollarVariableInstrumentationTest extends AgentTestRunner {

@Override
protected void configurePreAgent() {
injectSysConfig('dd.iast.enabled', 'true')
}

void 'test freemarker process'() {
given:
final module = Mock(XssModule)
InstrumentationBridge.registerIastModule(module)

final Configuration cfg = new Configuration()
final Template template = new Template("test", new StringReader("test \${$stringExpression}"), cfg)
final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper())
rootDataModel.put(stringExpression, expression)

when:
template.process(rootDataModel, Mock(FileWriter))

then:
1 * module.onXss(_, _, _)

where:
stringExpression | expression
"test" | "test"
}
}
Loading

0 comments on commit 2de3ffb

Please sign in to comment.