Skip to content

Commit d36ef9a

Browse files
MariovidoMariovido
authored
Add propagation to StringBuilder substring methods (#7980)
1 parent df51f63 commit d36ef9a

File tree

7 files changed

+132
-33
lines changed

7 files changed

+132
-33
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public void onStringConcatFactory(
185185
@Override
186186
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
187187
public void onStringSubSequence(
188-
@Nonnull String self, int beginIndex, int endIndex, @Nullable CharSequence result) {
188+
@Nonnull CharSequence self, int beginIndex, int endIndex, @Nullable CharSequence result) {
189189
if (self == result || !canBeTainted(result)) {
190190
return;
191191
}

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

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -481,37 +481,42 @@ class StringModuleTest extends IastModuleImplTestBase {
481481
}
482482
483483
where:
484-
self | beginIndex | endIndex | expected
485-
"==>0123<==" | 0 | 4 | "==>0123<=="
486-
"0123==>456<==78" | 0 | 5 | "0123==>4<=="
487-
"01==>234<==5==>678<==90" | 0 | 8 | "01==>234<==5==>67<=="
488-
"==>0123<==" | 0 | 3 | "==>012<=="
489-
"==>0123<==" | 1 | 4 | "==>123<=="
490-
"==>0123<==" | 1 | 3 | "==>12<=="
491-
"0123==>456<==78" | 1 | 8 | "123==>456<==7"
492-
"0123==>456<==78" | 0 | 4 | "0123"
493-
"0123==>456<==78" | 7 | 9 | "78"
494-
"0123==>456<==78" | 1 | 5 | "123==>4<=="
495-
"0123==>456<==78" | 1 | 6 | "123==>45<=="
496-
"0123==>456<==78" | 4 | 7 | "==>456<=="
497-
"0123==>456<==78" | 6 | 8 | "==>6<==7"
498-
"0123==>456<==78" | 5 | 8 | "==>56<==7"
499-
"0123==>456<==78" | 4 | 6 | "==>45<=="
500-
"01==>234<==5==>678<==90" | 1 | 10 | "1==>234<==5==>678<==9"
501-
"01==>234<==5==>678<==90" | 1 | 2 | "1"
502-
"01==>234<==5==>678<==90" | 5 | 6 | "5"
503-
"01==>234<==5==>678<==90" | 9 | 10 | "9"
504-
"01==>234<==5==>678<==90" | 1 | 4 | "1==>23<=="
505-
"01==>234<==5==>678<==90" | 2 | 4 | "==>23<=="
506-
"01==>234<==5==>678<==90" | 2 | 5 | "==>234<=="
507-
"01==>234<==5==>678<==90" | 1 | 8 | "1==>234<==5==>67<=="
508-
"01==>234<==5==>678<==90" | 2 | 8 | "==>234<==5==>67<=="
509-
"01==>234<==5==>678<==90" | 2 | 9 | "==>234<==5==>678<=="
510-
"01==>234<==5==>678<==90" | 5 | 8 | "5==>67<=="
511-
"01==>234<==5==>678<==90" | 6 | 8 | "==>67<=="
512-
"01==>234<==5==>678<==90" | 6 | 9 | "==>678<=="
513-
"01==>234<==5==>678<==90" | 4 | 9 | "==>4<==5==>678<=="
514-
"01==>234<==5==>678<==90" | 4 | 8 | "==>4<==5==>67<=="
484+
self | beginIndex | endIndex | expected
485+
"==>0123<==" | 0 | 4 | "==>0123<=="
486+
"0123==>456<==78" | 0 | 5 | "0123==>4<=="
487+
"01==>234<==5==>678<==90" | 0 | 8 | "01==>234<==5==>67<=="
488+
"==>0123<==" | 0 | 3 | "==>012<=="
489+
"==>0123<==" | 1 | 4 | "==>123<=="
490+
"==>0123<==" | 1 | 3 | "==>12<=="
491+
"0123==>456<==78" | 1 | 8 | "123==>456<==7"
492+
"0123==>456<==78" | 0 | 4 | "0123"
493+
"0123==>456<==78" | 7 | 9 | "78"
494+
"0123==>456<==78" | 1 | 5 | "123==>4<=="
495+
"0123==>456<==78" | 1 | 6 | "123==>45<=="
496+
"0123==>456<==78" | 4 | 7 | "==>456<=="
497+
"0123==>456<==78" | 6 | 8 | "==>6<==7"
498+
"0123==>456<==78" | 5 | 8 | "==>56<==7"
499+
"0123==>456<==78" | 4 | 6 | "==>45<=="
500+
"01==>234<==5==>678<==90" | 1 | 10 | "1==>234<==5==>678<==9"
501+
"01==>234<==5==>678<==90" | 1 | 2 | "1"
502+
"01==>234<==5==>678<==90" | 5 | 6 | "5"
503+
"01==>234<==5==>678<==90" | 9 | 10 | "9"
504+
"01==>234<==5==>678<==90" | 1 | 4 | "1==>23<=="
505+
"01==>234<==5==>678<==90" | 2 | 4 | "==>23<=="
506+
"01==>234<==5==>678<==90" | 2 | 5 | "==>234<=="
507+
"01==>234<==5==>678<==90" | 1 | 8 | "1==>234<==5==>67<=="
508+
"01==>234<==5==>678<==90" | 2 | 8 | "==>234<==5==>67<=="
509+
"01==>234<==5==>678<==90" | 2 | 9 | "==>234<==5==>678<=="
510+
"01==>234<==5==>678<==90" | 5 | 8 | "5==>67<=="
511+
"01==>234<==5==>678<==90" | 6 | 8 | "==>67<=="
512+
"01==>234<==5==>678<==90" | 6 | 9 | "==>678<=="
513+
"01==>234<==5==>678<==90" | 4 | 9 | "==>4<==5==>678<=="
514+
"01==>234<==5==>678<==90" | 4 | 8 | "==>4<==5==>67<=="
515+
sb("==>0123<==") | 0 | 4 | "==>0123<=="
516+
sb("0123==>456<==78") | 0 | 5 | "0123==>4<=="
517+
sb("01==>234<==5==>678<==90") | 0 | 8 | "01==>234<==5==>67<=="
518+
sb("0123==>456<==78") | 4 | 6 | "==>45<=="
519+
sb("01==>234<==5==>678<==90") | 4 | 8 | "==>4<==5==>67<=="
515520
}
516521
517522
void 'onStringJoin without null delimiter or elements (#delimiter, #elements)'() {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ class TaintUtils {
7474
new String(s.replace(OPEN_MARK, "").replace(CLOSE_MARK, ""))
7575
}
7676

77+
static String getStringFromTaintFormat(final Appendable appendable) {
78+
if (appendable == null) {
79+
return null
80+
}
81+
getStringFromTaintFormat(appendable.toString())
82+
}
83+
7784
static <E> E taint(final TaintedObjects tos, final E value) {
7885
if (value instanceof String) {
7986
return addFromTaintFormat(tos, value as String)

‎dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,37 @@ public static String afterToString(
101101
}
102102
return result;
103103
}
104+
105+
@CallSite.After("java.lang.String java.lang.StringBuilder.substring(int)")
106+
public static String afterSubstring(
107+
@CallSite.This final CharSequence self,
108+
@CallSite.Argument final int beginIndex,
109+
@CallSite.Return final String result) {
110+
final StringModule module = InstrumentationBridge.STRING;
111+
if (module != null) {
112+
try {
113+
module.onStringSubSequence(self, beginIndex, self.length(), result);
114+
} catch (final Throwable e) {
115+
module.onUnexpectedException("afterSubstring threw", e);
116+
}
117+
}
118+
return result;
119+
}
120+
121+
@CallSite.After("java.lang.String java.lang.StringBuilder.substring(int, int)")
122+
public static String afterSubstring(
123+
@CallSite.This final CharSequence self,
124+
@CallSite.Argument final int beginIndex,
125+
@CallSite.Argument final int endIndex,
126+
@CallSite.Return final String result) {
127+
final StringModule module = InstrumentationBridge.STRING;
128+
if (module != null) {
129+
try {
130+
module.onStringSubSequence(self, beginIndex, endIndex, result);
131+
} catch (final Throwable e) {
132+
module.onUnexpectedException("afterSubstring threw", e);
133+
}
134+
}
135+
return result;
136+
}
104137
}

‎dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,42 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
174174
ex.stackTrace.find { it.className == StringBuilderCallSite.name } == null
175175
}
176176
177+
def 'test string builder substring call site'() {
178+
setup:
179+
final iastModule = Mock(StringModule)
180+
InstrumentationBridge.registerIastModule(iastModule)
181+
182+
when:
183+
final result = TestStringBuilderSuite.substring(param, beginIndex)
184+
185+
then:
186+
result == expected
187+
1 * iastModule.onStringSubSequence(param, beginIndex, param.length(), expected)
188+
0 * _
189+
190+
where:
191+
param | beginIndex | expected
192+
sb('012345') | 1 | '12345'
193+
}
194+
195+
def 'test string builder substring with endIndex call site'() {
196+
setup:
197+
final iastModule = Mock(StringModule)
198+
InstrumentationBridge.registerIastModule(iastModule)
199+
200+
when:
201+
final result = TestStringBuilderSuite.substring(param, beginIndex, endIndex)
202+
203+
then:
204+
result == expected
205+
1 * iastModule.onStringSubSequence(param, beginIndex, endIndex, expected)
206+
0 * _
207+
208+
where:
209+
param | beginIndex | endIndex | expected
210+
sb('012345') | 1 | 5 | '1234'
211+
}
212+
177213
private static class BrokenToString {
178214
@Override
179215
String toString() {
@@ -186,4 +222,8 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
186222
super(message)
187223
}
188224
}
225+
226+
private static StringBuilder sb(final String string) {
227+
return new StringBuilder(string)
228+
}
189229
}

‎dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,18 @@ public String plus(final Object... items) {
7676
LOGGER.debug("After string builder toString {}", result);
7777
return result;
7878
}
79+
80+
public static String substring(StringBuilder self, int beginIndex, int endIndex) {
81+
LOGGER.debug("Before string builder substring {} from {} to {}", self, beginIndex, endIndex);
82+
final String result = self.substring(beginIndex, endIndex);
83+
LOGGER.debug("After string builder substring {}", result);
84+
return result;
85+
}
86+
87+
public static String substring(StringBuilder self, int beginIndex) {
88+
LOGGER.debug("Before string builder substring {} from {}", self, beginIndex);
89+
final String result = self.substring(beginIndex);
90+
LOGGER.debug("After string builder substring {}", result);
91+
return result;
92+
}
7993
}

‎internal-api/src/main/java/datadog/trace/api/iast/propagation/StringModule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ void onStringConcatFactory(
2323
@Nonnull int[] recipeOffsets);
2424

2525
void onStringSubSequence(
26-
@Nonnull String self, int beginIndex, int endIndex, @Nullable CharSequence result);
26+
@Nonnull CharSequence self, int beginIndex, int endIndex, @Nullable CharSequence result);
2727

2828
void onStringJoin(
2929
@Nullable String result, @Nonnull CharSequence delimiter, @Nonnull CharSequence[] elements);

0 commit comments

Comments
 (0)