From 7dec401d65bf9a247a47e4d4df0293c808c236cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Thu, 18 Apr 2024 10:04:58 +0200 Subject: [PATCH] Refactor propagation module to include two different APIs for strings and objects --- .../propagation/PropagationModuleImpl.java | 643 +++++++++++++----- .../propagation/BaseCodecModuleTest.groovy | 7 +- .../IastRequestContextPreparationTrait.groovy | 11 +- .../iast/PathMatcherInstrumentation.java | 15 +- .../helpers/TaintSingleParameterFunction.java | 4 +- .../iast/helpers/TaintParametersFunction.java | 4 +- .../java/lang/StringReaderCallSite.java | 6 +- .../java/lang/StringCallSite.java | 4 +- ...actParamValueExtractorInstrumentation.java | 2 +- .../jersey/AbstractStringReaderAdvice.java | 2 +- .../JSONObjectUtilsInstrumentation.java | 2 +- .../json/JSONArrayInstrumentation.java | 32 +- .../json/JSONObjectInstrumentation.java | 32 +- .../iast/PathMatcherInstrumentation.java | 17 +- .../iast/helpers/TaintParametersFunction.java | 4 +- .../helpers/TaintSingleParameterFunction.java | 4 +- .../resteasy/CookieParamInjectorAdvice.java | 4 +- .../resteasy/FormParamInjectorAdvice.java | 4 +- .../resteasy/HeaderParamInjectorAdvice.java | 20 +- .../resteasy/PathParamInjectorAdvice.java | 4 +- .../resteasy/QueryParamInjectorAdvice.java | 4 +- .../HttpServletResponseInstrumentation.java | 6 +- .../StreamUtilsInstrumentation.java | 2 +- .../vertx_3_4/core/BufferInstrumentation.java | 30 +- .../iast/propagation/PropagationModule.java | 177 ++++- 25 files changed, 734 insertions(+), 306 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java index ec07035c38c..96a8cc9451e 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java @@ -10,7 +10,6 @@ import com.datadog.iast.taint.Ranges; import com.datadog.iast.taint.TaintedObject; import com.datadog.iast.taint.TaintedObjects; -import com.datadog.iast.taint.Tainteds; import com.datadog.iast.util.ObjectVisitor; import com.datadog.iast.util.Ranged; import datadog.trace.api.Config; @@ -18,50 +17,75 @@ import datadog.trace.api.iast.Taintable; import datadog.trace.api.iast.propagation.PropagationModule; import java.lang.ref.WeakReference; -import java.lang.reflect.Array; import java.util.function.Predicate; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.jetbrains.annotations.Contract; +@SuppressWarnings("DuplicatedCode") public class PropagationModuleImpl implements PropagationModule { /** Prevent copy of values bigger than this threshold */ private static final int MAX_VALUE_LENGTH = Config.get().getIastTruncationMaxValueLength(); @Override - public void taint(@Nullable final Object target, final byte origin) { + public void taint(@Nullable Object target, byte origin) { taint(target, origin, null); } @Override - public void taint( - @Nullable final Object target, final byte origin, @Nullable final CharSequence name) { + public void taint(@Nullable IastContext ctx, @Nullable Object target, byte origin) { + taint(ctx, target, origin, null); + } + + @Override + public void taint(@Nullable String target, byte origin) { + taint(target, origin, null); + } + + @Override + public void taint(@Nullable IastContext ctx, @Nullable String target, byte origin) { + taint(ctx, target, origin, null); + } + + @Override + public void taint(@Nullable Object target, byte origin, @Nullable CharSequence name) { taint(target, origin, name, target); } @Override public void taint( - @Nullable final Object target, - final byte origin, - @Nullable final CharSequence name, - @Nullable final Object value) { - if (!canBeTainted(target)) { - return; - } - taint(LazyContext.build(), target, origin, name, value); + @Nullable IastContext ctx, + @Nullable Object target, + byte origin, + @Nullable CharSequence name) { + taint(ctx, target, origin, name, target); + } + + @Override + public void taint(@Nullable String target, byte origin, @Nullable CharSequence name) { + taint(target, origin, name, target); } @Override public void taint( - @Nullable final IastContext ctx, @Nullable final Object target, final byte origin) { - taint(ctx, target, origin, null); + @Nullable IastContext ctx, + @Nullable String target, + byte origin, + @Nullable CharSequence name) { + taint(ctx, target, origin, name, target); } @Override public void taint( @Nullable final Object target, final byte origin, final int start, final int length) { - taint(LazyContext.build(), target, origin, start, length); + if (target == null || length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taint(ctx, target, origin, start, length); } @Override @@ -71,21 +95,58 @@ public void taint( final byte origin, final int start, final int length) { - if (!canBeTainted(target) || length == 0) { + if (ctx == null || target == null || length == 0) { return; } + final TaintedObjects to = ctx.getTaintedObjects(); final Range range = new Range(start, length, newSource(target, origin, null, target), NOT_MARKED); - internalTaint(ctx, target, new Range[] {range}, NOT_MARKED); + internalTaint(to, target, new Range[] {range}, NOT_MARKED); + } + + @Override + public void taint( + @Nullable final String target, final byte origin, final int start, final int length) { + if (target == null || length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taint(ctx, target, origin, start, length); } @Override public void taint( @Nullable final IastContext ctx, + @Nullable final String target, + final byte origin, + final int start, + final int length) { + if (ctx == null || target == null || length == 0) { + return; + } + final TaintedObjects to = ctx.getTaintedObjects(); + final Range range = + new Range(start, length, newSource(target, origin, null, target), NOT_MARKED); + internalTaint(to, target, new Range[] {range}, NOT_MARKED); + } + + @Override + public void taint( @Nullable final Object target, final byte origin, - @Nullable final CharSequence name) { - taint(ctx, target, origin, name, target); + @Nullable final CharSequence name, + @Nullable final Object value) { + if (target == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taint(ctx, target, origin, name, value); } @Override @@ -95,48 +156,124 @@ public void taint( final byte origin, @Nullable final CharSequence name, @Nullable final Object value) { - if (!canBeTainted(target)) { + if (ctx == null || target == null) { return; } - internalTaint(ctx, target, newSource(target, origin, name, value), NOT_MARKED); + final TaintedObjects to = ctx.getTaintedObjects(); + internalTaint(to, target, newSource(target, origin, name, value), NOT_MARKED); + } + + @Override + public void taint( + @Nullable final String target, + final byte origin, + @Nullable final CharSequence name, + @Nullable final CharSequence value) { + if (target == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taint(ctx, target, origin, name, value); + } + + @Override + public void taint( + @Nullable final IastContext ctx, + @Nullable final String target, + final byte origin, + @Nullable final CharSequence name, + @Nullable final CharSequence value) { + if (ctx == null || target == null) { + return; + } + final TaintedObjects to = ctx.getTaintedObjects(); + internalTaint(to, target, newSource(target, origin, name, value), NOT_MARKED); + } + + @Override + public void taintIfTainted(@Nullable Object target, @Nullable Object input) { + taintIfTainted(target, input, false, NOT_MARKED); + } + + @Override + public void taintIfTainted( + @Nullable IastContext ctx, @Nullable Object target, @Nullable Object input) { + taintIfTainted(ctx, target, input, false, NOT_MARKED); } @Override - public void taintIfTainted(@Nullable final Object target, @Nullable final Object input) { + public void taintIfTainted(@Nullable String target, @Nullable Object input) { taintIfTainted(target, input, false, NOT_MARKED); } + @Override + public void taintIfTainted( + @Nullable IastContext ctx, @Nullable String target, @Nullable Object input) { + taintIfTainted(ctx, target, input, false, NOT_MARKED); + } + @Override public void taintIfTainted( @Nullable final Object target, @Nullable final Object input, boolean keepRanges, int mark) { - if (!canBeTainted(target) || !canBeTainted(input)) { + if (target == null || input == null) { return; } - taintIfTainted(LazyContext.build(), target, input, keepRanges, mark); + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfTainted(ctx, target, input, keepRanges, mark); } @Override public void taintIfTainted( @Nullable final IastContext ctx, @Nullable final Object target, - @Nullable final Object input) { - taintIfTainted(ctx, target, input, false, NOT_MARKED); + @Nullable final Object input, + boolean keepRanges, + int mark) { + if (ctx == null || target == null || input == null) { + return; + } + final TaintedObjects to = ctx.getTaintedObjects(); + if (keepRanges) { + internalTaint(to, target, getRanges(to, input), mark); + } else { + internalTaint(to, target, highestPrioritySource(to, input), mark); + } + } + + @Override + public void taintIfTainted( + @Nullable final String target, @Nullable final Object input, boolean keepRanges, int mark) { + if (target == null || input == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfTainted(ctx, target, input, keepRanges, mark); } @Override public void taintIfTainted( @Nullable final IastContext ctx, - @Nullable final Object target, + @Nullable final String target, @Nullable final Object input, boolean keepRanges, int mark) { - if (!canBeTainted(target) || !canBeTainted(input)) { + if (ctx == null || target == null || input == null) { return; } + final TaintedObjects to = ctx.getTaintedObjects(); if (keepRanges) { - internalTaint(ctx, target, getRanges(ctx, input), mark); + internalTaint(to, target, getRanges(to, input), mark); } else { - internalTaint(ctx, target, highestPrioritySource(ctx, input), mark); + internalTaint(to, target, highestPrioritySource(to, input), mark); } } @@ -148,7 +285,14 @@ public void taintIfTainted( final int length, boolean keepRanges, int mark) { - taintIfTainted(LazyContext.build(), target, input, start, length, keepRanges, mark); + if (target == null || input == null || length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfTainted(ctx, target, input, start, length, keepRanges, mark); } @Override @@ -160,10 +304,11 @@ public void taintIfTainted( final int length, boolean keepRanges, int mark) { - if (!canBeTainted(target) || !canBeTainted(input) || length == 0) { + if (ctx == null || target == null || input == null || length == 0) { return; } - final Range[] ranges = getRanges(ctx, input); + final TaintedObjects to = ctx.getTaintedObjects(); + final Range[] ranges = getRanges(to, input); if (ranges == null || ranges.length == 0) { return; } @@ -172,28 +317,114 @@ public void taintIfTainted( return; } if (keepRanges) { - internalTaint(ctx, target, intersection, mark); + internalTaint(to, target, intersection, mark); } else { final Range range = highestPriorityRange(intersection); - internalTaint(ctx, target, range.getSource(), mark); + internalTaint(to, target, range.getSource(), mark); } } @Override public void taintIfTainted( - @Nullable final Object target, @Nullable final Object input, final byte origin) { - taintIfTainted(target, input, origin, null); + @Nullable final String target, + @Nullable final Object input, + final int start, + final int length, + boolean keepRanges, + int mark) { + if (target == null || input == null || length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfTainted(ctx, target, input, start, length, keepRanges, mark); } @Override public void taintIfTainted( - @Nullable final Object target, + @Nullable final IastContext ctx, + @Nullable final String target, @Nullable final Object input, - final byte origin, - @Nullable final CharSequence name) { + final int start, + final int length, + boolean keepRanges, + int mark) { + if (ctx == null || target == null || input == null || length == 0) { + return; + } + final TaintedObjects to = ctx.getTaintedObjects(); + final Range[] ranges = getRanges(to, input); + if (ranges == null || ranges.length == 0) { + return; + } + final Range[] intersection = Ranges.intersection(Ranged.build(start, length), ranges); + if (intersection == null || intersection.length == 0) { + return; + } + if (keepRanges) { + internalTaint(to, target, intersection, mark); + } else { + final Range range = highestPriorityRange(intersection); + internalTaint(to, target, range.getSource(), mark); + } + } + + @Override + public void taintIfTainted(@Nullable Object target, @Nullable Object input, byte origin) { + taintIfTainted(target, input, origin, null, target); + } + + @Override + public void taintIfTainted( + @Nullable IastContext ctx, @Nullable Object target, @Nullable Object input, byte origin) { + taintIfTainted(ctx, target, input, origin, null, target); + } + + @Override + public void taintIfTainted(@Nullable String target, @Nullable Object input, byte origin) { + taintIfTainted(target, input, origin, null, target); + } + + @Override + public void taintIfTainted( + @Nullable IastContext ctx, @Nullable String target, @Nullable Object input, byte origin) { + taintIfTainted(ctx, target, input, origin, null, target); + } + + @Override + public void taintIfTainted( + @Nullable Object target, @Nullable Object input, byte origin, @Nullable CharSequence name) { + taintIfTainted(target, input, origin, name, target); + } + + @Override + public void taintIfTainted( + @Nullable IastContext ctx, + @Nullable Object target, + @Nullable Object input, + byte origin, + @Nullable CharSequence name) { + taintIfTainted(ctx, target, input, origin, name, target); + } + + @Override + public void taintIfTainted( + @Nullable String target, @Nullable Object input, byte origin, @Nullable CharSequence name) { taintIfTainted(target, input, origin, name, target); } + @Override + public void taintIfTainted( + @Nullable IastContext ctx, + @Nullable String target, + @Nullable Object input, + byte origin, + @Nullable CharSequence name) { + taintIfTainted(ctx, target, input, origin, name, target); + } + @Override public void taintIfTainted( @Nullable final Object target, @@ -201,10 +432,14 @@ public void taintIfTainted( final byte origin, @Nullable final CharSequence name, @Nullable final Object value) { - if (!canBeTainted(target) || !canBeTainted(input)) { + if (target == null || input == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { return; } - taintIfTainted(LazyContext.build(), target, input, origin, name, value); + taintIfTainted(ctx, target, input, origin, name, value); } @Override @@ -212,80 +447,150 @@ public void taintIfTainted( @Nullable final IastContext ctx, @Nullable final Object target, @Nullable final Object input, - final byte origin) { - taintIfTainted(ctx, target, input, origin, null); + final byte origin, + @Nullable final CharSequence name, + @Nullable final Object value) { + if (ctx == null || target == null || input == null) { + return; + } + if (isTainted(ctx, target)) { + final TaintedObjects to = ctx.getTaintedObjects(); + internalTaint(to, target, newSource(target, origin, name, value), NOT_MARKED); + } } @Override public void taintIfTainted( - @Nullable final IastContext ctx, - @Nullable final Object target, + @Nullable final String target, @Nullable final Object input, final byte origin, - @Nullable final CharSequence name) { - taintIfTainted(ctx, target, input, origin, name, target); + @Nullable final CharSequence name, + @Nullable final Object value) { + if (target == null || input == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfTainted(ctx, target, input, origin, name, value); } @Override public void taintIfTainted( @Nullable final IastContext ctx, - @Nullable final Object target, + @Nullable final String target, @Nullable final Object input, final byte origin, @Nullable final CharSequence name, @Nullable final Object value) { - if (!canBeTainted(target) || !canBeTainted(input)) { + if (ctx == null || target == null || input == null) { return; } if (isTainted(ctx, input)) { - internalTaint(ctx, target, newSource(target, origin, name, value), NOT_MARKED); + final TaintedObjects to = ctx.getTaintedObjects(); + internalTaint(to, target, newSource(target, origin, name, value), NOT_MARKED); } } @Override - public void taintIfAnyTainted(@Nullable final Object target, @Nullable final Object[] inputs) { + public void taintIfAnyTainted(@Nullable Object target, @Nullable Object[] inputs) { taintIfAnyTainted(target, inputs, false, NOT_MARKED); } + @Override + public void taintIfAnyTainted( + @Nullable IastContext ctx, @Nullable Object target, @Nullable Object[] inputs) { + taintIfAnyTainted(ctx, target, inputs, false, NOT_MARKED); + } + + @Override + public void taintIfAnyTainted(@Nullable String target, @Nullable Object[] inputs) { + taintIfAnyTainted(target, inputs, false, NOT_MARKED); + } + + @Override + public void taintIfAnyTainted( + @Nullable IastContext ctx, @Nullable String target, @Nullable Object[] inputs) { + taintIfAnyTainted(ctx, target, inputs, false, NOT_MARKED); + } + @Override public void taintIfAnyTainted( @Nullable final Object target, @Nullable final Object[] inputs, final boolean keepRanges, final int mark) { - if (!canBeTainted(target) || !canBeTainted(inputs)) { + if (target == null || inputs == null || inputs.length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { return; } - taintIfAnyTainted(LazyContext.build(), target, inputs, keepRanges, mark); + taintIfAnyTainted(ctx, target, inputs, keepRanges, mark); } @Override public void taintIfAnyTainted( @Nullable final IastContext ctx, @Nullable final Object target, - @Nullable final Object[] inputs) { - taintIfAnyTainted(ctx, target, inputs, false, NOT_MARKED); + @Nullable final Object[] inputs, + final boolean keepRanges, + final int mark) { + if (ctx == null || target == null || inputs == null || inputs.length == 0) { + return; + } + final TaintedObjects to = ctx.getTaintedObjects(); + if (keepRanges) { + final Range[] ranges = getRangesInArray(to, inputs); + if (ranges != null) { + internalTaint(to, target, ranges, mark); + } + } else { + final Source source = highestPrioritySourceInArray(to, inputs); + if (source != null) { + internalTaint(to, target, source, mark); + } + } + } + + @Override + public void taintIfAnyTainted( + @Nullable final String target, + @Nullable final Object[] inputs, + final boolean keepRanges, + final int mark) { + if (target == null || inputs == null || inputs.length == 0) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + taintIfAnyTainted(ctx, target, inputs, keepRanges, mark); } @Override public void taintIfAnyTainted( @Nullable final IastContext ctx, - @Nullable final Object target, + @Nullable final String target, @Nullable final Object[] inputs, final boolean keepRanges, final int mark) { - if (!canBeTainted(target) || !canBeTainted(inputs)) { + if (ctx == null || target == null || inputs == null || inputs.length == 0) { return; } + final TaintedObjects to = ctx.getTaintedObjects(); if (keepRanges) { - final Range[] ranges = getRangesInArray(ctx, inputs); + final Range[] ranges = getRangesInArray(to, inputs); if (ranges != null) { - internalTaint(ctx, target, ranges, mark); + internalTaint(to, target, ranges, mark); } } else { - final Source source = highestPrioritySourceInArray(ctx, inputs); + final Source source = highestPrioritySourceInArray(to, inputs); if (source != null) { - internalTaint(ctx, target, source, mark); + internalTaint(to, target, source, mark); } } } @@ -293,10 +598,14 @@ public void taintIfAnyTainted( @Override public int taintDeeply( @Nullable final Object target, final byte origin, final Predicate> classFilter) { - if (!canBeTainted(target)) { + if (target == null) { return 0; } - return taintDeeply(LazyContext.build(), target, origin, classFilter); + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return 0; + } + return taintDeeply(ctx, target, origin, classFilter); } @Override @@ -305,15 +614,12 @@ public int taintDeeply( @Nullable final Object target, final byte origin, final Predicate> classFilter) { - if (!canBeTainted(target)) { - return 0; - } - final TaintedObjects to = getTaintedObjects(ctx); - if (to == null) { + if (ctx == null || target == null) { return 0; } + final TaintedObjects to = ctx.getTaintedObjects(); if (target instanceof CharSequence) { - internalTaint(ctx, target, newSource(target, origin, null, target), NOT_MARKED); + internalTaint(to, target, newSource(target, origin, null, target), NOT_MARKED); return 1; } else { final TaintingVisitor visitor = new TaintingVisitor(to, origin); @@ -325,27 +631,42 @@ public int taintDeeply( @Nullable @Override public Taintable.Source findSource(@Nullable final Object target) { - return target == null ? null : findSource(LazyContext.build(), target); + if (target == null) { + return null; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return null; + } + return findSource(ctx, target); } @Nullable @Override public Taintable.Source findSource( @Nullable final IastContext ctx, @Nullable final Object target) { - if (target == null) { + if (ctx == null || target == null) { return null; } - return highestPrioritySource(ctx, target); + final TaintedObjects to = ctx.getTaintedObjects(); + return highestPrioritySource(to, target); } @Override public boolean isTainted(@Nullable final Object target) { - return target != null && isTainted(LazyContext.build(), target); + if (target == null) { + return false; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return false; + } + return isTainted(ctx, target); } @Override public boolean isTainted(@Nullable final IastContext ctx, @Nullable final Object target) { - return target != null && findSource(ctx, target) != null; + return ctx != null && target != null && findSource(ctx, target) != null; } /** @@ -353,10 +674,24 @@ public boolean isTainted(@Nullable final IastContext ctx, @Nullable final Object * properties */ private static Source newSource( - @Nonnull final Object tainted, + @Nonnull final Object target, final byte origin, @Nullable final CharSequence name, @Nullable final Object value) { + final Object sourceValue = sourceReference(target, value, true); + final Object sourceName = name == value ? sourceValue : sourceReference(target, name, false); + return new Source(origin, sourceName, sourceValue); + } + + /** + * Ensures that the reference is not kept due to a strong reference via the name or value + * properties + */ + private static Source newSource( + @Nonnull final String tainted, + final byte origin, + @Nullable final CharSequence name, + @Nullable final CharSequence value) { final Object sourceValue = sourceReference(tainted, value, true); final Object sourceName = name == value ? sourceValue : sourceReference(tainted, name, false); return new Source(origin, sourceName, sourceValue); @@ -384,38 +719,11 @@ private static Object sourceReference( return value ? PROPAGATION_PLACEHOLDER : null; } - @Contract("null -> false") - private static boolean canBeTainted(@Nullable final Object target) { - if (target == null) { - return false; - } - if (target instanceof CharSequence) { - return Tainteds.canBeTainted((CharSequence) target); - } - if (target.getClass().isArray()) { - return Array.getLength(target) > 0; - } - return true; - } - - @Contract("null -> false") - private static boolean canBeTainted(@Nullable final Object[] target) { - if (target == null || target.length == 0) { - return false; - } - return true; - } - - @Nullable - private static TaintedObjects getTaintedObjects(@Nullable final IastContext ctx) { - return ctx == null ? null : ctx.getTaintedObjects(); - } - @Nullable private static Range[] getRangesInArray( - final @Nullable IastContext ctx, final @Nonnull Object[] objects) { + final @Nonnull TaintedObjects to, final @Nonnull Object[] objects) { for (final Object object : objects) { - final Range[] ranges = getRanges(ctx, object); + final Range[] ranges = getRanges(to, object); if (ranges != null) { return ranges; } @@ -424,28 +732,24 @@ private static Range[] getRangesInArray( } @Nullable - private static Range[] getRanges(final @Nullable IastContext ctx, final @Nonnull Object object) { + private static Range[] getRanges(final @Nonnull TaintedObjects to, final @Nonnull Object object) { if (object instanceof Taintable) { - final Source source = highestPrioritySource(ctx, object); + final Source source = highestPrioritySource(to, object); if (source == null) { return null; } else { return new Range[] {new Range(0, Integer.MAX_VALUE, source, NOT_MARKED)}; } } - final TaintedObjects to = getTaintedObjects(ctx); - if (to == null) { - return null; - } final TaintedObject tainted = to.get(object); return tainted == null ? null : tainted.getRanges(); } @Nullable private static Source highestPrioritySourceInArray( - final @Nullable IastContext ctx, final @Nonnull Object[] objects) { + final @Nonnull TaintedObjects to, final @Nonnull Object[] objects) { for (final Object object : objects) { - final Source source = highestPrioritySource(ctx, object); + final Source source = highestPrioritySource(to, object); if (source != null) { return source; } @@ -455,17 +759,17 @@ private static Source highestPrioritySourceInArray( @Nullable private static Source highestPrioritySource( - final @Nullable IastContext ctx, final @Nonnull Object object) { + final @Nonnull TaintedObjects to, final @Nonnull Object object) { if (object instanceof Taintable) { return (Source) ((Taintable) object).$$DD$getSource(); } else { - final Range[] ranges = getRanges(ctx, object); + final Range[] ranges = getRanges(to, object); return ranges != null && ranges.length > 0 ? highestPriorityRange(ranges).getSource() : null; } } private static void internalTaint( - @Nullable final IastContext ctx, + @Nonnull final TaintedObjects to, @Nonnull final Object value, @Nullable Source source, int mark) { @@ -475,10 +779,6 @@ private static void internalTaint( if (value instanceof Taintable) { ((Taintable) value).$$DD$setSource(source); } else { - final TaintedObjects to = getTaintedObjects(ctx); - if (to == null) { - return; - } if (value instanceof CharSequence) { source = source.attachValue((CharSequence) value); to.taint(value, Ranges.forCharSequence((CharSequence) value, source, mark)); @@ -489,7 +789,19 @@ private static void internalTaint( } private static void internalTaint( - @Nullable final IastContext ctx, + @Nonnull final TaintedObjects to, + @Nonnull final String value, + @Nullable Source source, + int mark) { + if (source == null) { + return; + } + source = source.attachValue(value); + to.taint(value, Ranges.forCharSequence(value, source, mark)); + } + + private static void internalTaint( + @Nonnull final TaintedObjects to, @Nonnull final Object value, @Nullable Range[] ranges, final int mark) { @@ -502,25 +814,43 @@ private static void internalTaint( taintable.$$DD$setSource(ranges[0].getSource()); } } else { - final TaintedObjects to = getTaintedObjects(ctx); - if (to != null) { - if (value instanceof CharSequence) { - ranges = attachSourceValue(ranges, (CharSequence) value); - } - ranges = markRanges(ranges, mark); - final TaintedObject tainted = to.get(value); - if (tainted == null) { - // taint new value - to.taint(value, ranges); - } else { - // append ranges - final Range[] newRanges = Ranges.mergeRangesSorted(tainted.getRanges(), ranges); - tainted.setRanges(newRanges); - } + if (value instanceof CharSequence) { + ranges = attachSourceValue(ranges, (CharSequence) value); + } + ranges = markRanges(ranges, mark); + final TaintedObject tainted = to.get(value); + if (tainted != null) { + // append ranges + final Range[] newRanges = Ranges.mergeRangesSorted(tainted.getRanges(), ranges); + tainted.setRanges(newRanges); + } else { + // taint new value + to.taint(value, ranges); } } } + private static void internalTaint( + @Nonnull final TaintedObjects to, + @Nonnull final String value, + @Nullable Range[] ranges, + final int mark) { + if (ranges == null || ranges.length == 0) { + return; + } + ranges = attachSourceValue(ranges, value); + ranges = markRanges(ranges, mark); + final TaintedObject tainted = to.get(value); + if (tainted != null) { + // append ranges + final Range[] newRanges = Ranges.mergeRangesSorted(tainted.getRanges(), ranges); + tainted.setRanges(newRanges); + } else { + // taint new value + to.taint(value, ranges); + } + } + @Nonnull private static Range[] markRanges(@Nonnull final Range[] ranges, final int mark) { if (mark == NOT_MARKED) { @@ -549,33 +879,6 @@ private static Range[] attachSourceValue( : Ranges.forCharSequence(value, newSource, range.getMarks()); } - private static class LazyContext implements IastContext { - - private boolean fetched; - @Nullable private IastContext delegate; - - @Nullable - private IastContext getDelegate() { - if (!fetched) { - fetched = true; - delegate = IastContext.Provider.get(); - } - return delegate; - } - - public static IastContext build() { - return new LazyContext(); - } - - @SuppressWarnings("unchecked") - @Nonnull - @Override - public TaintedObjects getTaintedObjects() { - final IastContext delegate = getDelegate(); - return delegate == null ? TaintedObjects.NoOp.INSTANCE : delegate.getTaintedObjects(); - } - } - private static class TaintingVisitor implements ObjectVisitor.Visitor { private final TaintedObjects taintedObjects; @@ -592,12 +895,10 @@ private TaintingVisitor(@Nonnull final TaintedObjects taintedObjects, final byte public ObjectVisitor.State visit(@Nonnull final String path, @Nonnull final Object value) { if (value instanceof CharSequence) { final CharSequence charSequence = (CharSequence) value; - if (canBeTainted(charSequence)) { - final Source source = newSource(charSequence, origin, path, charSequence); - count++; - taintedObjects.taint( - charSequence, Ranges.forCharSequence(charSequence, source, NOT_MARKED)); - } + final Source source = newSource(charSequence, origin, path, charSequence); + count++; + taintedObjects.taint( + charSequence, Ranges.forCharSequence(charSequence, source, NOT_MARKED)); } return CONTINUE; } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/BaseCodecModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/BaseCodecModuleTest.groovy index b555ee6b87c..9c847a93875 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/BaseCodecModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/BaseCodecModuleTest.groovy @@ -25,7 +25,7 @@ abstract class BaseCodecModuleTest extends IastModuleImplTestBase { } } - void '#method null or empty'() { + void '#method null'() { when: module.&"$method".call(args.toArray()) @@ -35,15 +35,10 @@ abstract class BaseCodecModuleTest extends IastModuleImplTestBase { where: method | args 'onUrlDecode' | ['test', 'utf-8', null] - 'onUrlDecode' | ['test', 'utf-8', ''] 'onStringGetBytes' | ['test', 'utf-8', null] - 'onStringGetBytes' | ['test', 'utf-8', [] as byte[]] 'onStringFromBytes' | ['test'.bytes, 0, 2, 'utf-8', null] - 'onStringFromBytes' | ['test'.bytes, 0, 2, 'utf-8', ''] 'onBase64Encode' | ['test'.bytes, null] - 'onBase64Encode' | ['test'.bytes, [] as byte[]] 'onBase64Decode' | ['test'.bytes, null] - 'onBase64Decode' | ['test'.bytes, [] as byte[]] } void '#method no context'() { diff --git a/dd-java-agent/agent-iast/src/testFixtures/groovy/com/datadog/iast/test/IastRequestContextPreparationTrait.groovy b/dd-java-agent/agent-iast/src/testFixtures/groovy/com/datadog/iast/test/IastRequestContextPreparationTrait.groovy index 6e0b849f6ca..353cf322d07 100644 --- a/dd-java-agent/agent-iast/src/testFixtures/groovy/com/datadog/iast/test/IastRequestContextPreparationTrait.groovy +++ b/dd-java-agent/agent-iast/src/testFixtures/groovy/com/datadog/iast/test/IastRequestContextPreparationTrait.groovy @@ -69,9 +69,14 @@ trait IastRequestContextPreparationTrait { return tainted } - private final static Logger LOGGER = LoggerFactory.getLogger("map tainted objects") - static { - ((ch.qos.logback.classic.Logger) LOGGER).level = ch.qos.logback.classic.Level.DEBUG + private final static Logger LOGGER = withLogger("map tainted objects") + + private static Logger withLogger(final String name) { + final logger = LoggerFactory.getLogger(name) + if (logger instanceof ch.qos.logback.classic.Logger) { + ((ch.qos.logback.classic.Logger) logger).level = ch.qos.logback.classic.Level.DEBUG + } + return logger } private static void logTaint(Object o) { diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/PathMatcherInstrumentation.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/PathMatcherInstrumentation.java index 30175d95f3c..49e53af9813 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/PathMatcherInstrumentation.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/PathMatcherInstrumentation.java @@ -56,24 +56,23 @@ static void onExit( return; } + scala.Tuple1 tuple = (scala.Tuple1) extractions; + Object value = tuple._1(); + PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null) { + if (module == null || !(value instanceof String)) { return; } - - scala.Tuple1 tuple = (scala.Tuple1) extractions; - Object value = tuple._1(); + final String stringValue = (String) value; final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); // in the test, 4 instances of PathMatcher$Match are created, all with the same value - if (module.isTainted(ctx, value)) { + if (module.isTainted(ctx, stringValue)) { return; } - if (value instanceof String) { - module.taint(ctx, value, SourceTypes.REQUEST_PATH_PARAMETER); - } + module.taint(ctx, stringValue, SourceTypes.REQUEST_PATH_PARAMETER); } } } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/helpers/TaintSingleParameterFunction.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/helpers/TaintSingleParameterFunction.java index b329facad35..3cbc98c3dbc 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/helpers/TaintSingleParameterFunction.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/helpers/TaintSingleParameterFunction.java @@ -50,11 +50,11 @@ public Tuple1 apply(Tuple1 v1) { while (iterator.hasNext()) { Object o = iterator.next(); if (o instanceof String) { - mod.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else if (value instanceof String) { - mod.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } return v1; diff --git a/dd-java-agent/instrumentation/akka-http-10.2-iast/src/main/java/datadog/trace/instrumentation/akkahttp102/iast/helpers/TaintParametersFunction.java b/dd-java-agent/instrumentation/akka-http-10.2-iast/src/main/java/datadog/trace/instrumentation/akkahttp102/iast/helpers/TaintParametersFunction.java index 89e328c88bd..887fc36c144 100644 --- a/dd-java-agent/instrumentation/akka-http-10.2-iast/src/main/java/datadog/trace/instrumentation/akkahttp102/iast/helpers/TaintParametersFunction.java +++ b/dd-java-agent/instrumentation/akka-http-10.2-iast/src/main/java/datadog/trace/instrumentation/akkahttp102/iast/helpers/TaintParametersFunction.java @@ -41,11 +41,11 @@ public Tuple1 apply(Tuple1 v1) { while (iterator.hasNext()) { Object o = iterator.next(); if (o instanceof String) { - mod.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else if (value instanceof String) { - mod.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } return v1; diff --git a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/StringReaderCallSite.java b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/StringReaderCallSite.java index 09f203efa60..91d310ed2ab 100644 --- a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/StringReaderCallSite.java +++ b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/StringReaderCallSite.java @@ -18,7 +18,11 @@ public static StringReader afterInit( @CallSite.Return @Nonnull final StringReader result) { final PropagationModule propagationModule = InstrumentationBridge.PROPAGATION; if (propagationModule != null) { - propagationModule.taintIfTainted(result, params[0]); + try { + propagationModule.taintIfTainted(result, params[0]); + } catch (Throwable e) { + propagationModule.onUnexpectedException("afterInit threw", e); + } } 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 index c48ec0e350c..06b55699f6a 100644 --- 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 @@ -123,7 +123,7 @@ public static String aroundJoin( final StringModule module = InstrumentationBridge.STRING; if (module != null) { try { - module.onStringJoin(result, delimiter, copy.toArray(new CharSequence[copy.size()])); + module.onStringJoin(result, delimiter, copy.toArray(new CharSequence[0])); } catch (final Throwable e) { module.onUnexpectedException("afterSubSequence threw", e); } @@ -412,7 +412,7 @@ public static String[] afterSplit( public static String[] afterSplitWithLimit( @CallSite.This @Nonnull final String self, @CallSite.Argument(0) @Nonnull final String regex, - @CallSite.Argument(1) @Nonnull final int pos, + @CallSite.Argument(1) final int pos, @CallSite.Return @Nonnull final String[] result) { final StringModule module = InstrumentationBridge.STRING; if (module != null) { diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java index 9fe244af34e..124d641cb74 100644 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java @@ -54,7 +54,7 @@ public static void onExit( final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - module.taint(ctx, result, ThreadLocalSourceType.get(), parameterName); + module.taint(ctx, (String) result, ThreadLocalSourceType.get(), parameterName); } } } diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java index 575fbed1100..89274cfee34 100644 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java @@ -21,7 +21,7 @@ public static void onExit( final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - module.taint(ctx, result, SourceTypes.REQUEST_PARAMETER_VALUE); + module.taint(ctx, (String) result, SourceTypes.REQUEST_PARAMETER_VALUE); } } } diff --git a/dd-java-agent/instrumentation/jose-jwt/src/main/java/datadog/trace/instrumentation/josejwt/JSONObjectUtilsInstrumentation.java b/dd-java-agent/instrumentation/jose-jwt/src/main/java/datadog/trace/instrumentation/josejwt/JSONObjectUtilsInstrumentation.java index 2af1be5219f..baae4938e38 100644 --- a/dd-java-agent/instrumentation/jose-jwt/src/main/java/datadog/trace/instrumentation/josejwt/JSONObjectUtilsInstrumentation.java +++ b/dd-java-agent/instrumentation/jose-jwt/src/main/java/datadog/trace/instrumentation/josejwt/JSONObjectUtilsInstrumentation.java @@ -56,7 +56,7 @@ public static void onEnter( if (value instanceof String) { // TODO: We could represent this source more accurately, perhaps tracking the original // source, or using a special name. - module.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); + module.taint(ctx, (String) value, SourceTypes.REQUEST_HEADER_VALUE, name); } } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 6ac7e8956e7..09cdd91054d 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -14,6 +14,8 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; +import org.json.JSONArray; +import org.json.JSONObject; @AutoService(InstrumenterModule.class) public class JSONArrayInstrumentation extends InstrumenterModule.Iast @@ -60,15 +62,18 @@ public static class GetAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - if (result instanceof Integer - || result instanceof Long - || result instanceof Double - || result instanceof Boolean) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { return; } final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null && result != null && result instanceof String) { - iastModule.taintIfTainted(result, self); + if (iastModule != null) { + if (isString) { + iastModule.taintIfTainted((String) result, self); + } else { + iastModule.taintIfTainted(result, self); + } } } } @@ -77,15 +82,18 @@ public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - if (result instanceof Integer - || result instanceof Long - || result instanceof Double - || result instanceof Boolean) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { return; } final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null && result != null) { - iastModule.taintIfTainted(result, self); + if (iastModule != null) { + if (isString) { + iastModule.taintIfTainted((String) result, self); + } else { + iastModule.taintIfTainted(result, self); + } } } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index cae6f15906d..89c7b0f1712 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -14,6 +14,8 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; +import org.json.JSONArray; +import org.json.JSONObject; @AutoService(InstrumenterModule.class) public class JSONObjectInstrumentation extends InstrumenterModule.Iast @@ -62,15 +64,18 @@ public static class GetAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - if (result instanceof Integer - || result instanceof Long - || result instanceof Double - || result instanceof Boolean) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { return; } final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null && result != null) { - iastModule.taintIfTainted(result, self); + if (iastModule != null) { + if (isString) { + iastModule.taintIfTainted((String) result, self); + } else { + iastModule.taintIfTainted(result, self); + } } } } @@ -79,15 +84,18 @@ public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - if (result instanceof Integer - || result instanceof Long - || result instanceof Double - || result instanceof Boolean) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { return; } final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null && result != null) { - iastModule.taintIfTainted(result, self); + if (iastModule != null) { + if (isString) { + iastModule.taintIfTainted((String) result, self); + } else { + iastModule.taintIfTainted(result, self); + } } } } diff --git a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/PathMatcherInstrumentation.java b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/PathMatcherInstrumentation.java index c71c39b7ae3..50395cfadfe 100644 --- a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/PathMatcherInstrumentation.java +++ b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/PathMatcherInstrumentation.java @@ -56,24 +56,23 @@ static void onExit( return; } + scala.Tuple1 tuple = (scala.Tuple1) extractions; + Object value = tuple._1(); + PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null) { + if (module == null || !(value instanceof String)) { return; } + final String stringValue = (String) value; - scala.Tuple1 tuple = (scala.Tuple1) extractions; - Object value = tuple._1(); - - IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); // in the test, 4 instances of PathMatcher$Match are created, all with the same value - if (module.isTainted(ctx, value)) { + if (module.isTainted(ctx, stringValue)) { return; } - if (value instanceof String) { - module.taint(ctx, value, SourceTypes.REQUEST_PATH_PARAMETER); - } + module.taint(ctx, stringValue, SourceTypes.REQUEST_PATH_PARAMETER); } } } diff --git a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintParametersFunction.java b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintParametersFunction.java index ba805419050..e77667a6f72 100644 --- a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintParametersFunction.java +++ b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintParametersFunction.java @@ -44,11 +44,11 @@ public Tuple1 apply(Tuple1 v1) { while (iterator.hasNext()) { Object o = iterator.next(); if (o instanceof String) { - mod.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else if (value instanceof String) { - mod.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } return v1; diff --git a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintSingleParameterFunction.java b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintSingleParameterFunction.java index e22a8729e4d..710cca86977 100644 --- a/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintSingleParameterFunction.java +++ b/dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/iast/helpers/TaintSingleParameterFunction.java @@ -51,11 +51,11 @@ public Tuple1 apply(Tuple1 v1) { while (iterator.hasNext()) { Object o = iterator.next(); if (o instanceof String) { - mod.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else if (value instanceof String) { - mod.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + mod.taint(ctx, (String) value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } return v1; diff --git a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/CookieParamInjectorAdvice.java b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/CookieParamInjectorAdvice.java index 96fcbca06ca..81c7f536105 100644 --- a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/CookieParamInjectorAdvice.java +++ b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/CookieParamInjectorAdvice.java @@ -28,11 +28,11 @@ public static void onExit( Collection collection = (Collection) result; for (Object o : collection) { if (o instanceof String) { - module.taint(ctx, o, SourceTypes.REQUEST_COOKIE_VALUE, paramName); + module.taint(ctx, (String) o, SourceTypes.REQUEST_COOKIE_VALUE, paramName); } } } else { - module.taint(ctx, result, SourceTypes.REQUEST_COOKIE_VALUE, paramName); + module.taint(ctx, (String) result, SourceTypes.REQUEST_COOKIE_VALUE, paramName); } } } diff --git a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/FormParamInjectorAdvice.java b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/FormParamInjectorAdvice.java index 5c481697db6..9f1fff61394 100644 --- a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/FormParamInjectorAdvice.java +++ b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/FormParamInjectorAdvice.java @@ -28,11 +28,11 @@ public static void onExit( Collection collection = (Collection) result; for (Object o : collection) { if (o instanceof String) { - module.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + module.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else { - module.taint(ctx, result, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + module.taint(ctx, (String) result, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } diff --git a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/HeaderParamInjectorAdvice.java b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/HeaderParamInjectorAdvice.java index 25c46edb3d5..e3a9799739a 100644 --- a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/HeaderParamInjectorAdvice.java +++ b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/HeaderParamInjectorAdvice.java @@ -23,20 +23,16 @@ public static void onExit( if (result instanceof String || result instanceof Collection) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - if (result instanceof Collection) { - Collection collection = (Collection) result; - for (Object o : collection) { - if (o instanceof String) { - module.taint(ctx, o, SourceTypes.REQUEST_HEADER_VALUE, paramName); - } + IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (result instanceof Collection) { + Collection collection = (Collection) result; + for (Object o : collection) { + if (o instanceof String) { + module.taint(ctx, (String) o, SourceTypes.REQUEST_HEADER_VALUE, paramName); } - } else { - module.taint(ctx, result, SourceTypes.REQUEST_HEADER_VALUE, paramName); } - } catch (final Throwable e) { - module.onUnexpectedException("HeaderParamInjectorAdvice.onExit threw", e); + } else { + module.taint(ctx, (String) result, SourceTypes.REQUEST_HEADER_VALUE, paramName); } } } diff --git a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/PathParamInjectorAdvice.java b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/PathParamInjectorAdvice.java index 96fd981adc1..1469d06996c 100644 --- a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/PathParamInjectorAdvice.java +++ b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/PathParamInjectorAdvice.java @@ -28,11 +28,11 @@ public static void onExit( Collection collection = (Collection) result; for (Object o : collection) { if (o instanceof String) { - module.taint(ctx, o, SourceTypes.REQUEST_PATH_PARAMETER, paramName); + module.taint(ctx, (String) o, SourceTypes.REQUEST_PATH_PARAMETER, paramName); } } } else { - module.taint(ctx, result, SourceTypes.REQUEST_PATH_PARAMETER, paramName); + module.taint(ctx, (String) result, SourceTypes.REQUEST_PATH_PARAMETER, paramName); } } } diff --git a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/QueryParamInjectorAdvice.java b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/QueryParamInjectorAdvice.java index c5c07b27ef2..1271746f377 100644 --- a/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/QueryParamInjectorAdvice.java +++ b/dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/QueryParamInjectorAdvice.java @@ -28,11 +28,11 @@ public static void onExit( Collection collection = (Collection) result; for (Object o : collection) { if (o instanceof String) { - module.taint(ctx, o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + module.taint(ctx, (String) o, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } else { - module.taint(ctx, result, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); + module.taint(ctx, (String) result, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); } } } diff --git a/dd-java-agent/instrumentation/servlet-common/src/main/java/datadog/trace/instrumentation/servlet/HttpServletResponseInstrumentation.java b/dd-java-agent/instrumentation/servlet-common/src/main/java/datadog/trace/instrumentation/servlet/HttpServletResponseInstrumentation.java index 79213fde074..906d50b9da1 100644 --- a/dd-java-agent/instrumentation/servlet-common/src/main/java/datadog/trace/instrumentation/servlet/HttpServletResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet-common/src/main/java/datadog/trace/instrumentation/servlet/HttpServletResponseInstrumentation.java @@ -120,10 +120,8 @@ public static class EncodeURLAdvice { @Propagation public static void onExit(@Advice.Argument(0) final String url, @Advice.Return String encoded) { final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - if (null != url && !url.isEmpty() && null != encoded && !encoded.isEmpty()) { - module.taintIfTainted(encoded, url); - } + if (encoded != null && module != null) { + module.taintIfTainted(encoded, url); } } } diff --git a/dd-java-agent/instrumentation/spring-core/src/main/java/datadog/trace/instrumentation/springcore/StreamUtilsInstrumentation.java b/dd-java-agent/instrumentation/spring-core/src/main/java/datadog/trace/instrumentation/springcore/StreamUtilsInstrumentation.java index 232faaa742b..f2177536e00 100644 --- a/dd-java-agent/instrumentation/spring-core/src/main/java/datadog/trace/instrumentation/springcore/StreamUtilsInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-core/src/main/java/datadog/trace/instrumentation/springcore/StreamUtilsInstrumentation.java @@ -49,7 +49,7 @@ public static class SpringAdvice { public static void checkReturnedObject( @Advice.Return String string, @Advice.Argument(0) final InputStream in) { final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (in != null && string != null && !string.isEmpty()) { + if (string != null && module != null) { module.taintIfTainted(string, in); } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/BufferInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/BufferInstrumentation.java index 2ce7e7c9de9..5a6e946344a 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/BufferInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/BufferInstrumentation.java @@ -60,47 +60,35 @@ public void methodAdvice(final MethodTransformer transformer) { } public static class ToStringAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void get(@Advice.This final Object self, @Advice.Return final String result) { final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taintIfTainted(result, self); - } catch (final Throwable e) { - module.onUnexpectedException("toString threw", e); - } + if (result != null && module != null) { + module.taintIfTainted(result, self); } } } public static class GetByteBuffAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void get(@Advice.This final Object self, @Advice.Return final Object result) { final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taintIfTainted(result, self); - } catch (final Throwable e) { - module.onUnexpectedException("getByteBuf threw", e); - } + if (result != null && module != null) { + module.taintIfTainted(result, self); } } } public static class AppendBufferAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Propagation public static void get( @Advice.Argument(0) final Object buffer, @Advice.Return final Object result) { final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taintIfTainted(result, buffer); - } catch (final Throwable e) { - module.onUnexpectedException("appendBuffer threw", e); - } + if (result != null && module != null) { + module.taintIfTainted(result, buffer); } } } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java b/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java index 250a3962a69..670c65286a5 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java @@ -19,27 +19,28 @@ public interface PropagationModule extends IastModule { */ void taint(@Nullable IastContext ctx, @Nullable Object target, byte origin); - /** @see #taint(IastContext, Object, byte, int, int) */ - void taint(@Nullable Object target, byte origin, int start, int length); + /** @see #taint(IastContext, String, byte) */ + void taint(@Nullable String target, byte origin); + + /** @see #taint(IastContext, Object, byte) */ + void taint(@Nullable IastContext ctx, @Nullable String target, byte origin); /** - * Taints the object with a source with the selected origin, range and no name. If target is a - * char sequence it will be used as value. - * - *

If the value is already tainted this method will append a new range. + * Taints the object with a source with the selected origin and name, if target is a char sequence + * it will be used as value */ void taint( - @Nullable IastContext ctx, @Nullable Object target, byte origin, int start, int length); + @Nullable IastContext ctx, @Nullable Object target, byte origin, @Nullable CharSequence name); /** @see #taint(IastContext, Object, byte, CharSequence) */ void taint(@Nullable Object target, byte origin, @Nullable CharSequence name); - /** - * Taints the object with a source with the selected origin and name, if target is a char sequence - * it will be used as value - */ + /** @see #taint(IastContext, Object, byte, CharSequence) */ void taint( - @Nullable IastContext ctx, @Nullable Object target, byte origin, @Nullable CharSequence name); + @Nullable IastContext ctx, @Nullable String target, byte origin, @Nullable CharSequence name); + + /** @see #taint(IastContext, String, byte, CharSequence) */ + void taint(@Nullable String target, byte origin, @Nullable CharSequence name); /** @see #taint(IastContext, Object, byte, CharSequence, Object) */ void taint( @@ -53,6 +54,40 @@ void taint( @Nullable CharSequence name, @Nullable Object value); + /** @see #taint(IastContext, String, byte, CharSequence, CharSequence) */ + void taint( + @Nullable String target, + byte origin, + @Nullable CharSequence name, + @Nullable CharSequence value); + + /** @see #taint(IastContext, Object, byte, CharSequence, Object) */ + void taint( + @Nullable IastContext ctx, + @Nullable String target, + byte origin, + @Nullable CharSequence name, + @Nullable CharSequence value); + + /** @see #taint(IastContext, Object, byte, int, int) */ + void taint(@Nullable Object target, byte origin, int start, int length); + + /** + * Taints the object with a source with the selected origin, range and no name. If target is a + * char sequence it will be used as value. + * + *

If the value is already tainted this method will append a new range. + */ + void taint( + @Nullable IastContext ctx, @Nullable Object target, byte origin, int start, int length); + + /** @see #taint(IastContext, String, byte, int, int) */ + void taint(@Nullable String target, byte origin, int start, int length); + + /** @see #taint(IastContext, Object, byte, int, int) */ + void taint( + @Nullable IastContext ctx, @Nullable String target, byte origin, int start, int length); + /** @see #taintIfTainted(IastContext, Object, Object) */ void taintIfTainted(@Nullable Object target, @Nullable Object input); @@ -62,7 +97,45 @@ void taint( */ void taintIfTainted(@Nullable IastContext ctx, @Nullable Object target, @Nullable Object input); - /** @see #taintIfTainted(IastContext, Object, Object, int, int, boolean, int) */ + /** @see #taintIfTainted(IastContext, String, Object) */ + void taintIfTainted(@Nullable String target, @Nullable Object input); + + /** @see #taintIfTainted(IastContext, Object, Object) */ + void taintIfTainted(@Nullable IastContext ctx, @Nullable String target, @Nullable Object input); + + /** @see #taintIfTainted(IastContext, Object, Object, boolean, int) */ + void taintIfTainted( + @Nullable Object target, @Nullable Object input, boolean keepRanges, int mark); + + /** + * Taints the object only if the input value is tainted. It will try to reuse sources from the + * input value according to: + * + *

    + *
  • keepRanges=true will reuse the ranges from the input tainted value and mark them + *
  • keepRanges=false will use the highest priority source from the input ranges and mark it + *
+ */ + void taintIfTainted( + @Nullable IastContext ctx, + @Nullable Object target, + @Nullable Object input, + boolean keepRanges, + int mark); + + /** @see #taintIfTainted(IastContext, String, Object, boolean, int) */ + void taintIfTainted( + @Nullable String target, @Nullable Object input, boolean keepRanges, int mark); + + /** @see #taintIfTainted(IastContext, Object, Object, boolean, int) */ + void taintIfTainted( + @Nullable IastContext ctx, + @Nullable String target, + @Nullable Object input, + boolean keepRanges, + int mark); + + /** @see #taintIfTainted(IastContext, Object, Object, boolean, int) */ void taintIfTainted( @Nullable Object target, @Nullable Object input, @@ -89,23 +162,22 @@ void taintIfTainted( boolean keepRanges, int mark); - /** @see #taintIfTainted(IastContext, Object, Object, boolean, int) */ + /** @see #taintIfTainted(IastContext, String, Object, boolean, int) */ void taintIfTainted( - @Nullable Object target, @Nullable Object input, boolean keepRanges, int mark); + @Nullable String target, + @Nullable Object input, + int start, + int length, + boolean keepRanges, + int mark); - /** - * Taints the object only if the input value is tainted. It will try to reuse sources from the - * input value according to: - * - *
    - *
  • keepRanges=true will reuse the ranges from the input tainted value and mark them - *
  • keepRanges=false will use the highest priority source from the input ranges and mark it - *
- */ + /** @see #taintIfTainted(IastContext, Object, Object, boolean, int) */ void taintIfTainted( @Nullable IastContext ctx, - @Nullable Object target, + @Nullable String target, @Nullable Object input, + int start, + int length, boolean keepRanges, int mark); @@ -120,6 +192,13 @@ void taintIfTainted( void taintIfTainted( @Nullable IastContext ctx, @Nullable Object target, @Nullable Object input, byte origin); + /** @see #taintIfTainted(IastContext, String, Object, byte) */ + void taintIfTainted(@Nullable String target, @Nullable Object input, byte origin); + + /** @see #taintIfTainted(IastContext, Object, Object, byte) */ + void taintIfTainted( + @Nullable IastContext ctx, @Nullable String target, @Nullable Object input, byte origin); + /** @see #taintIfTainted(IastContext, Object, Object, byte, CharSequence) */ void taintIfTainted( @Nullable Object target, @Nullable Object input, byte origin, @Nullable CharSequence name); @@ -136,6 +215,18 @@ void taintIfTainted( byte origin, @Nullable CharSequence name); + /** @see #taintIfTainted(IastContext, String, Object, byte, CharSequence) */ + void taintIfTainted( + @Nullable String target, @Nullable Object input, byte origin, @Nullable CharSequence name); + + /** @see #taintIfTainted(IastContext, Object, Object, byte, CharSequence) */ + void taintIfTainted( + @Nullable IastContext ctx, + @Nullable String target, + @Nullable Object input, + byte origin, + @Nullable CharSequence name); + /** @see #taintIfTainted(IastContext, Object, Object, byte, CharSequence, Object) */ void taintIfTainted( @Nullable Object target, @@ -156,6 +247,23 @@ void taintIfTainted( @Nullable CharSequence name, @Nullable Object value); + /** @see #taintIfTainted(IastContext, String, Object, byte, CharSequence, Object) */ + void taintIfTainted( + @Nullable String target, + @Nullable Object input, + byte origin, + @Nullable CharSequence name, + @Nullable Object value); + + /** @see #taintIfTainted(IastContext, Object, Object, byte, CharSequence, Object) */ + void taintIfTainted( + @Nullable IastContext ctx, + @Nullable String target, + @Nullable Object input, + byte origin, + @Nullable CharSequence name, + @Nullable Object value); + /** @see #taintIfAnyTainted(IastContext, Object, Object[]) */ void taintIfAnyTainted(@Nullable Object target, @Nullable Object[] inputs); @@ -168,6 +276,13 @@ void taintIfTainted( void taintIfAnyTainted( @Nullable IastContext ctx, @Nullable Object target, @Nullable Object[] inputs); + /** @see #taintIfAnyTainted(IastContext, String, Object[]) */ + void taintIfAnyTainted(@Nullable String target, @Nullable Object[] inputs); + + /** @see #taintIfAnyTainted(IastContext, Object, Object[]) */ + void taintIfAnyTainted( + @Nullable IastContext ctx, @Nullable String target, @Nullable Object[] inputs); + /** @see #taintIfAnyTainted(IastContext, Object, Object[], boolean, int) */ void taintIfAnyTainted( @Nullable Object target, @Nullable Object[] inputs, boolean keepRanges, int mark); @@ -185,6 +300,18 @@ void taintIfAnyTainted( boolean keepRanges, int mark); + /** @see #taintIfAnyTainted(IastContext, String, Object[], boolean, int) */ + void taintIfAnyTainted( + @Nullable String target, @Nullable Object[] inputs, boolean keepRanges, int mark); + + /** @see #taintIfAnyTainted(IastContext, Object, Object[], boolean, int) */ + void taintIfAnyTainted( + @Nullable IastContext ctx, + @Nullable String target, + @Nullable Object[] inputs, + boolean keepRanges, + int mark); + /** @see #taintDeeply(IastContext, Object, byte, Predicate) */ int taintDeeply(@Nullable Object target, byte origin, Predicate> classFilter);