Skip to content

Commit dda483e

Browse files
MariovidoMariovido
authored
Add taint propagation to the String indent method (#7707)
1 parent fd84e8c commit dda483e

File tree

12 files changed

+358
-3
lines changed

12 files changed

+358
-3
lines changed

‎buildSrc/call-site-instrumentation-plugin/build.gradle.kts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ dependencies {
3434
compileOnly("com.google.code.findbugs", "jsr305", "3.0.2")
3535

3636
implementation("org.freemarker", "freemarker", "2.3.30")
37-
implementation("org.ow2.asm", "asm", "9.0")
38-
implementation("org.ow2.asm", "asm-tree", "9.0")
37+
implementation("org.ow2.asm", "asm", "9.7")
38+
implementation("org.ow2.asm", "asm-tree", "9.7")
3939
implementation("com.github.javaparser", "javaparser-symbol-solver-core", "3.24.4")
4040

4141
testImplementation("net.bytebuddy", "byte-buddy", "1.14.18")

‎dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,33 @@ public void onStringStrip(@Nonnull String self, @Nonnull String result, boolean
604604
}
605605
}
606606

607+
@Override
608+
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
609+
public void onIndent(@Nonnull String self, int indentation, @Nonnull String result) {
610+
if (self == result || !canBeTainted(self)) {
611+
return;
612+
}
613+
final IastContext ctx = IastContext.Provider.get();
614+
if (ctx == null) {
615+
return;
616+
}
617+
final TaintedObjects taintedObjects = ctx.getTaintedObjects();
618+
final TaintedObject taintedSelf = taintedObjects.get(self);
619+
if (taintedSelf == null) {
620+
return;
621+
}
622+
623+
final Range[] rangesSelf = taintedSelf.getRanges();
624+
if (rangesSelf.length == 0) {
625+
return;
626+
}
627+
628+
final Range[] newRanges = Ranges.forIndentation(self, indentation, rangesSelf);
629+
if (newRanges != null) {
630+
taintedObjects.taint(result, newRanges);
631+
}
632+
}
633+
607634
/**
608635
* Adds the tainted ranges belonging to the current parameter added via placeholder taking care of
609636
* an optional tainted placeholder.

‎dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.datadog.iast.util.HttpHeader;
99
import com.datadog.iast.util.RangeBuilder;
1010
import com.datadog.iast.util.Ranged;
11+
import com.datadog.iast.util.StringUtils;
1112
import datadog.trace.api.iast.SourceTypes;
1213
import javax.annotation.Nonnull;
1314
import javax.annotation.Nullable;
@@ -304,4 +305,125 @@ public static Range[] getNotMarkedRanges(@Nullable final Range[] ranges, final i
304305
public static Range copyWithPosition(final Range range, final int offset, final int length) {
305306
return new Range(offset, length, range.getSource(), range.getMarks());
306307
}
308+
309+
/** Returns a new array of ranges with the indentation applied to each line */
310+
public static Range[] forIndentation(
311+
String input, int indentation, final @Nonnull Range[] ranges) {
312+
final Range[] newRanges = new Range[ranges.length];
313+
int delimitersCount = 0;
314+
int offset = 0;
315+
int rangeStart = 0;
316+
int currentIndex = 0;
317+
int totalWhiteSpaces = 0;
318+
while (rangeStart < ranges.length || currentIndex < input.length() - 1) {
319+
final int[] delimiterIndex = getNextDelimiterIndex(input, currentIndex, currentIndex + 1);
320+
final int lineOffset = delimiterIndex[1] == 2 ? 1 : 0;
321+
int currentIndentation;
322+
// In case indentation is negative we need to check if it will take more than the first
323+
// non-white space character
324+
if (indentation < 0) {
325+
final int whiteSpaces =
326+
StringUtils.leadingWhitespaces(input, currentIndex, delimiterIndex[0]);
327+
currentIndentation = Math.max(indentation, -whiteSpaces) - totalWhiteSpaces;
328+
totalWhiteSpaces += whiteSpaces;
329+
} else {
330+
currentIndentation = indentation * ++delimitersCount;
331+
}
332+
currentIndentation -= offset;
333+
rangeStart =
334+
updateRangesWithIndentation(
335+
currentIndex,
336+
delimiterIndex[0] - 1,
337+
indentation,
338+
rangeStart,
339+
ranges,
340+
newRanges,
341+
currentIndentation,
342+
lineOffset);
343+
offset += lineOffset;
344+
currentIndex = delimiterIndex[0];
345+
}
346+
347+
return newRanges;
348+
}
349+
350+
/**
351+
* Returns two numbers in an array:
352+
*
353+
* <p>1. The index of the next delimiter (his last character)
354+
*
355+
* <p>2. The length of the delimiter ({@code 1} or {@code 2})
356+
*
357+
* <p>In case there is no delimiter, it will return the last index of the string and {@code 1}
358+
*
359+
* @param original is the original string
360+
* @param start is the start index of the substring
361+
* @param offset is to take into account the previous lines
362+
*/
363+
private static int[] getNextDelimiterIndex(
364+
@Nonnull final String original, final int start, final int offset) {
365+
for (int i = start; i < original.length(); i++) {
366+
final char c = original.charAt(i);
367+
if (c == '\n') {
368+
return new int[] {i + offset, 1};
369+
} else if (c == '\r') {
370+
if (i + 1 < original.length() && original.charAt(i + 1) == '\n') {
371+
return new int[] {i + 1 + offset, 2};
372+
}
373+
return new int[] {i + offset, 1};
374+
}
375+
}
376+
377+
return new int[] {original.length() - 1 + offset - start, 1};
378+
}
379+
380+
/**
381+
* Updates the {@code newRanges} array between the {@code start} index and the {@code end} index
382+
* and taking into account the {@code indentation}
383+
*
384+
* <p>The {@code rangeStart} is the index of the first range that will be checked
385+
*
386+
* <p>The {@code ranges} is the current ranges array
387+
*
388+
* <p>The {@code offset} is to take into account the normalization of the indent method
389+
*
390+
* <p>The {@code lineOffset} is to know if the line is being normalized
391+
*
392+
* @return the index of the first range that will be checked
393+
*/
394+
private static int updateRangesWithIndentation(
395+
int start,
396+
int end,
397+
int indentation,
398+
int rangeStart,
399+
final @Nonnull Range[] ranges,
400+
final @Nonnull Range[] newRanges,
401+
int offset,
402+
int lineOffset) {
403+
int i = rangeStart;
404+
while (i < ranges.length && ranges[i].getStart() <= end) {
405+
Range range = ranges[i];
406+
if (range.getStart() >= start) {
407+
final int newStart = range.getStart() + offset;
408+
int newLength = range.getLength();
409+
if (range.getStart() + range.getLength() > end) {
410+
newLength -= lineOffset;
411+
}
412+
newRanges[i] = new Range(newStart, newLength, range.getSource(), range.getMarks());
413+
} else if (range.getStart() + range.getLength() >= start) {
414+
final Range newRange = newRanges[i];
415+
final int newLength = newRange.getLength() + indentation;
416+
newRanges[i] =
417+
new Range(newRange.getStart(), newLength, newRange.getSource(), newRange.getMarks());
418+
}
419+
420+
if (range.getStart() + range.getLength() - 1 <= end) {
421+
rangeStart++;
422+
}
423+
424+
i++;
425+
}
426+
427+
return rangeStart;
428+
}
307429
}

‎dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,14 @@ public static String substringTrim(@Nonnull final String value, int start, int e
3838
}
3939
return start >= end ? "" : value.substring(start, end);
4040
}
41+
42+
/** Returns how many leading whitespaces are between the start and the end of the string */
43+
@Nonnull
44+
public static int leadingWhitespaces(@Nonnull final String value, int start, int end) {
45+
int whitespaces = start;
46+
while (whitespaces < end && Character.isWhitespace(value.charAt(whitespaces))) {
47+
whitespaces++;
48+
}
49+
return whitespaces;
50+
}
4151
}

‎dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,46 @@ class StringModuleTest extends IastModuleImplTestBase {
10561056
true | "" | ""
10571057
}
10581058
1059+
void 'test indent and make sure IastRequestContext is called'() {
1060+
given:
1061+
final taintedObjects = ctx.getTaintedObjects()
1062+
def self = addFromTaintFormat(taintedObjects, testString)
1063+
objectHolder.add(self)
1064+
1065+
and:
1066+
final result = getStringFromTaintFormat(expected)
1067+
objectHolder.add(expected)
1068+
final shouldBeTainted = fromTaintFormat(expected) != null
1069+
1070+
when:
1071+
module.onIndent(self, indentation, result)
1072+
1073+
then:
1074+
1 * tracer.activeSpan() >> span
1075+
def to = ctx.getTaintedObjects().get(result)
1076+
if (shouldBeTainted) {
1077+
assert to != null
1078+
assert to.get() == result
1079+
assert taintFormat(to.get() as String, to.getRanges()) == expected
1080+
} else {
1081+
assert to == null
1082+
}
1083+
where:
1084+
indentation | testString | expected
1085+
4 | "==>123<==\n12==>3<==" | " ==>123<==\n 12==>3<=="
1086+
4 | "==>123<==\r\n12==>3<==" | " ==>123<==\n 12==>3<=="
1087+
4 | "==>123\n1<==2==>3<==" | " ==>123\n 1<==2==>3<=="
1088+
4 | "==>123\r\n1<==2==>3<==" | " ==>123\n 1<==2==>3<=="
1089+
0 | "==>123<==\r\n==>123<==" | "==>123<==\n==>123<=="
1090+
0 | "==>123\r\n<====>123<==" | "==>123\n<====>123<=="
1091+
0 | "==>123<==\r==>123<==" | "==>123<==\n==>123<=="
1092+
0 | "==>123\r<====>123<==" | "==>123\n<====>123<=="
1093+
-4 | " ==>123<==\n 12==>3<==" | "==>123<==\n12==>3<=="
1094+
-4 | " ==>123<==\r\n 12==>3<==" | "==>123<==\n12==>3<=="
1095+
-4 | " ==>123\n 1<==2==>3<==" | "==>123\n1<==2==>3<=="
1096+
-4 | " ==>123\r\n 1<==2==>3<==" | "==>123\n1<==2==>3<=="
1097+
}
1098+
10591099
private static Date date(final String pattern, final String value) {
10601100
return new SimpleDateFormat(pattern).parse(value)
10611101
}

‎dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/RangesTest.groovy‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ class RangesTest extends DDSpecification {
289289
[REFERER] | [
290290
rangeWithSource(REQUEST_HEADER_VALUE, REFERER.name),
291291
rangeWithSource(REQUEST_HEADER_VALUE, LOCATION.name)
292-
] | true
292+
] | true
293293
}
294294

295295
void 'test intersection of ranges'() {
@@ -332,6 +332,30 @@ class RangesTest extends DDSpecification {
332332
[range(2, 2), range(6, 2)] | [range(0, 2), range(4, 2)] | [range(0, 2), range(2, 2), range(4, 2), range(6, 2)]
333333
}
334334

335+
void 'test forIndentation method'() {
336+
when:
337+
final result = Ranges.forIndentation(input, indentation, ranges as Range[])
338+
339+
then:
340+
final expectedArray = expected as Range[]
341+
result == expectedArray
342+
343+
where:
344+
input | indentation | ranges | expected
345+
// "123\n123" | 4 | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")] | [rangeWithSource(4, 3, (byte) 1, null, "123"), rangeWithSource(14, 1, (byte) 2, null, "3")]
346+
// "123\r\n123" | 4 | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(7, 1, (byte) 2, null, "3")] | [rangeWithSource(4, 3, (byte) 1, null, "123"), rangeWithSource(14, 1, (byte) 2, null, "3")]
347+
"123\n123" | 4 | [rangeWithSource(0, 5, (byte) 1, null, "123\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")] | [rangeWithSource(4, 9, (byte) 1, null, "123\n1"), rangeWithSource(14, 1, (byte) 2, null, "3")]
348+
// "123\r\n123" | 4 | [rangeWithSource(0, 6, (byte) 1, null, "123\r\n1"), rangeWithSource(7, 1, (byte) 2, null, "3")] | [rangeWithSource(4, 9, (byte) 1, null, "123\r\n1"), rangeWithSource(14, 1, (byte) 2, null, "3")]
349+
// "123\n123" | 0 | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")]
350+
// "123\r\n123" | 0 | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(7, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")]
351+
// "123\n123" | 0 | [rangeWithSource(0, 5, (byte) 1, null, "123\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 5, (byte) 1, null, "123\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")]
352+
// "123\r\n123" | 0 | [rangeWithSource(0, 6, (byte) 1, null, "123\r\n1"), rangeWithSource(7, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 5, (byte) 1, null, "123\r\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")]
353+
// " 123\n 123" | -4 | [rangeWithSource(4, 3, (byte) 1, null, "123"), rangeWithSource(14, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")]
354+
// " 123\r\n 123" | -4 | [rangeWithSource(4, 3, (byte) 1, null, "123"), rangeWithSource(15, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 3, (byte) 1, null, "123"), rangeWithSource(6, 1, (byte) 2, null, "3")]
355+
// " 123\n 123" | -4 | [rangeWithSource(4, 9, (byte) 1, null, "123\n1"), rangeWithSource(14, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 5, (byte) 1, null, "123\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")]
356+
// " 123\r\n 123" | -4 | [rangeWithSource(4, 10, (byte) 1, null, "123\r\n1"), rangeWithSource(15, 1, (byte) 2, null, "3")] | [rangeWithSource(0, 5, (byte) 1, null, "123\r\n1"), rangeWithSource(6, 1, (byte) 2, null, "3")]
357+
}
358+
335359

336360
Range[] rangesFromSpec(List<List<Object>> spec) {
337361
def ranges = new Range[spec.size()]
@@ -368,4 +392,8 @@ class RangesTest extends DDSpecification {
368392
Range range(final int start, final int length) {
369393
return new Range(start, length, new Source(REQUEST_HEADER_NAME, 'a', 'b'), NOT_MARKED)
370394
}
395+
396+
Range rangeWithSource(final int start, final int length, final byte source, final String name = 'name', final String value = 'value') {
397+
return new Range(start, length, new Source(source, name, value), NOT_MARKED)
398+
}
371399
}

‎dd-java-agent/instrumentation/java-lang/java-lang-17/build.gradle‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
plugins {
2+
id 'idea'
3+
}
4+
5+
ext {
6+
minJavaVersionForTests = JavaVersion.VERSION_17
7+
}
8+
9+
apply from: "$rootDir/gradle/java.gradle"
10+
apply plugin: 'call-site-instrumentation'
11+
12+
muzzle {
13+
pass {
14+
coreJdk()
15+
}
16+
}
17+
18+
idea {
19+
module {
20+
jdkName = '17'
21+
}
22+
}
23+
24+
csi {
25+
javaVersion = JavaLanguageVersion.of(17)
26+
}
27+
28+
addTestSuiteForDir('latestDepTest', 'test')
29+
30+
dependencies {
31+
testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')
32+
}
33+
34+
project.tasks.withType(AbstractCompile).configureEach {
35+
setJavaVersion(it, 17)
36+
if (it.name != 'compileCsiJava') {
37+
sourceCompatibility = JavaVersion.VERSION_17
38+
targetCompatibility = JavaVersion.VERSION_17
39+
if (it instanceof JavaCompile) {
40+
it.options.release.set(17)
41+
}
42+
}
43+
}

‎dd-java-agent/instrumentation/java-lang/java-lang-17/src/main/java/datadog/trace/instrumentation/java/lang/jdk17/StringCallSite.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package datadog.trace.instrumentation.java.lang.jdk17;
2+
3+
import datadog.trace.agent.tooling.csi.CallSite;
4+
import datadog.trace.api.iast.IastCallSites;
5+
import datadog.trace.api.iast.InstrumentationBridge;
6+
import datadog.trace.api.iast.Propagation;
7+
import datadog.trace.api.iast.propagation.StringModule;
8+
9+
@Propagation
10+
@CallSite(
11+
spi = IastCallSites.class,
12+
enabled = {"datadog.trace.api.iast.IastEnabledChecks", "isMajorJavaVersionAtLeast", "17"})
13+
public class StringCallSite {
14+
@CallSite.After("java.lang.String java.lang.String.indent(int)")
15+
public static String afterIndent(
16+
@CallSite.This final String self,
17+
@CallSite.Argument final int indentation,
18+
@CallSite.Return final String result) {
19+
final StringModule module = InstrumentationBridge.STRING;
20+
try {
21+
if (module != null) {
22+
module.onIndent(self, indentation, result);
23+
}
24+
} catch (final Throwable e) {
25+
module.onUnexpectedException("afterIndent threw", e);
26+
}
27+
return result;
28+
}
29+
}

‎dd-java-agent/instrumentation/java-lang/java-lang-17/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk17/StringCallSiteTest.groovy‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package datadog.trace.instrumentation.java.lang.jdk17
2+
3+
import datadog.trace.agent.test.AgentTestRunner
4+
import datadog.trace.api.iast.InstrumentationBridge
5+
import datadog.trace.api.iast.propagation.StringModule
6+
import foo.bar.TestStringJDK17Suite
7+
import spock.lang.Requires
8+
9+
@Requires({
10+
jvm.java17Compatible
11+
})
12+
class StringCallSiteTest extends AgentTestRunner {
13+
14+
@Override
15+
protected void configurePreAgent() {
16+
injectSysConfig("dd.iast.enabled", "true")
17+
}
18+
19+
def 'test string indent call site'() {
20+
setup:
21+
final iastModule = Mock(StringModule)
22+
InstrumentationBridge.registerIastModule(iastModule)
23+
24+
when:
25+
final result = TestStringJDK17Suite.stringIndent(input, indentation)
26+
27+
then:
28+
result == output
29+
1 * iastModule.onIndent(input, indentation, output)
30+
31+
where:
32+
input | indentation | output
33+
'Hello\nThis is a line' | 3 | ' Hello\n This is a line\n'
34+
}
35+
}

0 commit comments

Comments
 (0)