Skip to content

Commit

Permalink
Merge branch 'master' into sezen.leblay/APPSEC-55380-translateEscapes…
Browse files Browse the repository at this point in the history 
…-propagation
  • Loading branch information
@sezen-datadog
sezen-datadog authored Jan 14, 2025
2 parents 1037009 + 866fc61 commit 977bff8
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 151 deletions.
23 changes: 0 additions & 23 deletions ...er/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ public class CapturedContext implements ValueReferenceResolver {
private Map<String, CapturedValue> staticFields;
private Limits limits = Limits.DEFAULT;
private String thisClassName;
private String traceId;
private String spanId;
private long duration;
private final Map<String, Status> statusByProbeId = new LinkedHashMap<>();
private Map<String, CapturedValue> watches;
Expand Down Expand Up @@ -239,19 +237,6 @@ public void addStaticFields(CapturedValue[] values) {
}
}

public void addTraceId(CapturedValue capturedValue) {
traceId = extractStringId(capturedValue);
}

public void addSpanId(CapturedValue capturedValue) {
spanId = extractStringId(capturedValue);
}

private String extractStringId(CapturedValue capturedValue) {
Object value = capturedValue.getValue();
return value instanceof String ? (String) value : null;
}

public void setLimits(
int maxReferenceDepth, int maxCollectionSize, int maxLength, int maxFieldCount) {
this.limits = new Limits(maxReferenceDepth, maxCollectionSize, maxLength, maxFieldCount);
Expand Down Expand Up @@ -285,14 +270,6 @@ public String getThisClassName() {
return thisClassName;
}

public String getTraceId() {
return traceId;
}

public String getSpanId() {
return spanId;
}

/**
* 'Freeze' the context. The contained arguments, locals and fields are converted from their Java
* instance representation into the corresponding string value.
Expand Down
14 changes: 8 additions & 6 deletions ...va-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
*/
public class DebuggerTransformer implements ClassFileTransformer {
private static final Logger log = LoggerFactory.getLogger(DebuggerTransformer.class);
private static final String CANNOT_FIND_METHOD = "Cannot find method %s::%s";
private static final String CANNOT_FIND_METHOD = "Cannot find method %s::%s%s";
private static final String INSTRUMENTATION_FAILS = "Instrumentation fails for %s";
private static final String CANNOT_FIND_LINE = "No executable code was found at %s:L%s";
private static final Pattern COMMA_PATTERN = Pattern.compile(",");
Expand Down Expand Up @@ -536,20 +536,22 @@ private void handleInstrumentationResult(
private void reportLocationNotFound(
ProbeDefinition definition, String className, String methodName) {
if (methodName != null) {
reportErrorForAllProbes(singletonList(definition), CANNOT_FIND_METHOD, className, methodName);
String signature = definition.getWhere().getSignature();
signature = signature == null ? "" : signature;
String msg = String.format(CANNOT_FIND_METHOD, className, methodName, signature);
reportErrorForAllProbes(singletonList(definition), msg);
return;
}
// This is a line probe, so we don't report line not found because the line may be found later
// on a separate class files because probe was set on an inner/top-level class
}

private void reportInstrumentationFails(List<ProbeDefinition> definitions, String className) {
reportErrorForAllProbes(definitions, INSTRUMENTATION_FAILS, className, null);
String msg = String.format(INSTRUMENTATION_FAILS, className);
reportErrorForAllProbes(definitions, msg);
}

private void reportErrorForAllProbes(
List<ProbeDefinition> definitions, String format, String className, String location) {
String msg = String.format(format, className, location);
private void reportErrorForAllProbes(List<ProbeDefinition> definitions, String msg) {
DiagnosticMessage diagnosticMessage = new DiagnosticMessage(DiagnosticMessage.Kind.ERROR, msg);
for (ProbeDefinition definition : definitions) {
addDiagnostics(definition, singletonList(diagnosticMessage));
Expand Down
101 changes: 0 additions & 101 deletions ...ugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import static com.datadog.debugger.instrumentation.Types.CAPTURED_VALUE;
import static com.datadog.debugger.instrumentation.Types.CAPTURE_THROWABLE_TYPE;
import static com.datadog.debugger.instrumentation.Types.CLASS_TYPE;
import static com.datadog.debugger.instrumentation.Types.CORRELATION_ACCESS_TYPE;
import static com.datadog.debugger.instrumentation.Types.DEBUGGER_CONTEXT_TYPE;
import static com.datadog.debugger.instrumentation.Types.METHOD_LOCATION_TYPE;
import static com.datadog.debugger.instrumentation.Types.OBJECT_TYPE;
Expand All @@ -34,7 +33,6 @@
import com.datadog.debugger.sink.Snapshot;
import com.datadog.debugger.util.ClassFileLines;
import datadog.trace.api.Config;
import datadog.trace.bootstrap.debugger.CorrelationAccess;
import datadog.trace.bootstrap.debugger.Limits;
import datadog.trace.bootstrap.debugger.MethodLocation;
import datadog.trace.bootstrap.debugger.ProbeId;
Expand Down Expand Up @@ -778,8 +776,6 @@ private InsnList collectCapturedContext(Snapshot.Kind kind, AbstractInsnNode loc
// stack: [capturedcontext]
collectStaticFields(insnList);
// stack: [capturedcontext]
collectCorrelationInfo(insnList);
// stack: [capturedcontext]
/*
* It makes no sense collecting local variables for exceptions - the ones contributing to the exception
* are most likely to be outside of the scope in the exception handler block and there is no way to figure
Expand Down Expand Up @@ -1096,42 +1092,6 @@ private void collectStaticFields(InsnList insnList) {
// stack: [capturedcontext]
}

private void collectCorrelationInfo(InsnList insnList) {
// expected stack top: [capturedcontext]
/*
* We are cheating a bit with CorrelationAccess - utilizing the knowledge that it is a singleton loaded by the
* bootstrap class loader we can assume that the availability will not change during the app life time.
* As a side effect, we happen to initialize the access here and not from the injected code.
*/
boolean correlationAvailable = CorrelationAccess.instance().isAvailable();
if (isStatic && !correlationAvailable) {
// static method and no correlation info, no need to capture fields
return;
}
extractSpecialId(insnList, "dd.trace_id", "getTraceId", "addTraceId");
// stack: [capturedcontext]
extractSpecialId(insnList, "dd.span_id", "getSpanId", "addSpanId");
// stack: [capturedcontext]
}

private void extractSpecialId(
InsnList insnList, String fieldName, String getMethodName, String addMethodName) {
insnList.add(new InsnNode(Opcodes.DUP));
// stack: [capturedcontext, capturedcontext]
ldc(insnList, fieldName);
// stack: [capturedcontext, capturedcontext, name]
ldc(insnList, STRING_TYPE.getClassName());
// stack: [capturedcontext, capturedcontext, name, type_name]
invokeStatic(insnList, CORRELATION_ACCESS_TYPE, "instance", CORRELATION_ACCESS_TYPE);
// stack: [capturedcontext, capturedcontext, name, type_name, access]
invokeVirtual(insnList, CORRELATION_ACCESS_TYPE, getMethodName, STRING_TYPE);
// stack: [capturedcontext, capturedcontext, name, type_name, id]
addCapturedValueOf(insnList, limits);
// stack: [capturedcontext, capturedcontext, captured_value]
invokeVirtual(insnList, CAPTURED_CONTEXT_TYPE, addMethodName, Type.VOID_TYPE, CAPTURED_VALUE);
// stack: [capturedcontext]
}

private static boolean isAccessible(FieldNode fieldNode) {
Object value = fieldNode.value;
if (value instanceof Field) {
Expand All @@ -1140,67 +1100,6 @@ private static boolean isAccessible(FieldNode fieldNode) {
return true;
}

private static List<FieldNode> extractInstanceField(
ClassNode classNode, boolean isStatic, ClassLoader classLoader, Limits limits) {
List<FieldNode> results = new ArrayList<>();
if (CorrelationAccess.instance().isAvailable()) {
results.add(
new FieldNode(
Opcodes.ACC_PRIVATE, "dd.trace_id", STRING_TYPE.getDescriptor(), null, null));
results.add(
new FieldNode(
Opcodes.ACC_PRIVATE, "dd.span_id", STRING_TYPE.getDescriptor(), null, null));
}
if (isStatic) {
return results;
}
int fieldCount = 0;
for (FieldNode fieldNode : classNode.fields) {
if (isStaticField(fieldNode)) {
continue;
}
results.add(fieldNode);
fieldCount++;
if (fieldCount > limits.maxFieldCount) {
return results;
}
}
addInheritedFields(classNode, classLoader, limits, results, fieldCount);
return results;
}

private static void addInheritedFields(
ClassNode classNode,
ClassLoader classLoader,
Limits limits,
List<FieldNode> results,
int fieldCount) {
String superClassName = extractSuperClass(classNode);
while (!superClassName.equals(Object.class.getTypeName())) {
Class<?> clazz;
try {
clazz = Class.forName(superClassName, false, classLoader);
} catch (ClassNotFoundException ex) {
break;
}
for (Field field : clazz.getDeclaredFields()) {
if (isStaticField(field)) {
continue;
}
String desc = Type.getDescriptor(field.getType());
FieldNode fieldNode =
new FieldNode(field.getModifiers(), field.getName(), desc, null, field);
results.add(fieldNode);
fieldCount++;
if (fieldCount > limits.maxFieldCount) {
return;
}
}
clazz = clazz.getSuperclass();
superClassName = clazz.getTypeName();
}
}

private static List<FieldNode> extractStaticFields(
ClassNode classNode, ClassLoader classLoader, Limits limits) {
int fieldCount = 0;
Expand Down
15 changes: 5 additions & 10 deletions dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.squareup.moshi.Types;
import datadog.trace.api.Config;
import datadog.trace.bootstrap.debugger.CapturedContext;
import datadog.trace.bootstrap.debugger.CorrelationAccess;
import datadog.trace.bootstrap.debugger.DebuggerContext;
import datadog.trace.bootstrap.debugger.EvaluationError;
import datadog.trace.bootstrap.debugger.Limits;
Expand Down Expand Up @@ -549,25 +550,19 @@ protected boolean fillSnapshot(
LogStatus entryStatus = convertStatus(entryContext.getStatus(probeId.getEncodedId()));
LogStatus exitStatus = convertStatus(exitContext.getStatus(probeId.getEncodedId()));
String message = null;
String traceId = null;
String spanId = null;
switch (evaluateAt) {
case ENTRY:
case DEFAULT:
message = entryStatus.getMessage();
traceId = entryContext.getTraceId();
spanId = entryContext.getSpanId();
break;
case EXIT:
message = exitStatus.getMessage();
traceId = exitContext.getTraceId();
spanId = exitContext.getSpanId();
break;
}
boolean shouldCommit = false;
if (entryStatus.shouldSend() && exitStatus.shouldSend()) {
snapshot.setTraceId(traceId);
snapshot.setSpanId(spanId);
snapshot.setTraceId(CorrelationAccess.instance().getTraceId());
snapshot.setSpanId(CorrelationAccess.instance().getSpanId());
if (isCaptureSnapshot()) {
snapshot.setEntry(entryContext);
snapshot.setExit(exitContext);
Expand Down Expand Up @@ -643,8 +638,8 @@ public void commit(CapturedContext lineContext, int line) {
Snapshot snapshot = createSnapshot();
boolean shouldCommit = false;
if (status.shouldSend()) {
snapshot.setTraceId(lineContext.getTraceId());
snapshot.setSpanId(lineContext.getSpanId());
snapshot.setTraceId(CorrelationAccess.instance().getTraceId());
snapshot.setSpanId(CorrelationAccess.instance().getSpanId());
snapshot.setMessage(status.getMessage());
shouldCommit = true;
}
Expand Down
7 changes: 7 additions & 0 deletions dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ public interface VulnerabilityType {
/** A flag to indicate if the vulnerability is deduplicable. */
boolean isDeduplicable();

byte type();

static Builder type(final byte type) {
return new Builder(type);
}
Expand Down Expand Up @@ -191,6 +193,11 @@ public boolean isDeduplicable() {
return deduplicable;
}

@Override
public byte type() {
return type;
}

/** Useful for troubleshooting issues when vulns are serialized without moshi */
public String getName() {
return name();
Expand Down
25 changes: 25 additions & 0 deletions dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SinkModuleBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.datadog.iast.util.ObjectVisitor.State.CONTINUE;
import static com.datadog.iast.util.ObjectVisitor.State.EXIT;
import static datadog.trace.api.iast.VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK;

import com.datadog.iast.Dependencies;
import com.datadog.iast.Reporter;
Expand All @@ -22,6 +23,8 @@
import datadog.trace.api.Pair;
import datadog.trace.api.iast.IastContext;
import datadog.trace.api.iast.Taintable;
import datadog.trace.api.iast.telemetry.IastMetric;
import datadog.trace.api.iast.telemetry.IastMetricCollector;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.instrumentation.iastinstrumenter.IastExclusionTrie;
Expand Down Expand Up @@ -126,6 +129,7 @@ protected final Evidence checkInjection(
final TaintedObject tainted = origin == null ? null : to.get(origin);
if (origin != null && tainted != null) {
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
value = origin;
} else {
valueRanges = Ranges.forObject((Source) taintable.$$DD$getSource(), type.mark());
Expand All @@ -137,6 +141,7 @@ protected final Evidence checkInjection(
return null;
}
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
}

if (valueRanges == null || valueRanges.length == 0) {
Expand All @@ -160,6 +165,25 @@ protected final Evidence checkInjection(
return report(span, type, evidence, ranges, locationSupplier);
}

private void addSecurityControlMetrics(
@Nonnull final IastContext ctx,
@Nullable final Range[] valueRanges,
@Nonnull final Range[] taintedRanges,
@Nonnull final VulnerabilityType type) {
if ((valueRanges != null
&& valueRanges.length
!= 0) // ranges without the vulnerability mark implies vulnerability
|| taintedRanges.length == 0 // no tainted ranges
) {
return;
}
// check if there are tainted ranges without the security control mark
Range[] marked = Ranges.getNotMarkedRanges(taintedRanges, CUSTOM_SECURITY_CONTROL_MARK);
if (marked == null || marked.length == 0) {
IastMetricCollector.add(IastMetric.SUPPRESSED_VULNERABILITIES, type.type(), 1, ctx);
}
}

@Nullable
protected final Evidence checkInjection(final VulnerabilityType type, final Iterator<?> items) {
return checkInjection(type, items, null, null);
Expand Down Expand Up @@ -207,6 +231,7 @@ protected final Evidence checkInjection(
Range[] valueRanges = null;
if (tainted != null) {
valueRanges = Ranges.getNotMarkedRanges(tainted.getRanges(), type.mark());
addSecurityControlMetrics(ctx, valueRanges, tainted.getRanges(), type);
}
addToEvidence(type, evidence, ranges, value, valueRanges, evidenceBuilder);

Expand Down
Loading

0 comments on commit 977bff8

Please sign in to comment.