Skip to content

Commit 565afcd

Browse files
authored
Add propagation to String constructors with StringBuffer and StringBuilder (#7966)
1 parent b055376 commit 565afcd

File tree

6 files changed

+72
-5
lines changed

6 files changed

+72
-5
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
@@ -403,7 +403,7 @@ public void onStringTrim(@Nonnull final String self, @Nullable final String resu
403403
}
404404

405405
@Override
406-
public void onStringConstructor(@Nonnull String self, @Nonnull String result) {
406+
public void onStringConstructor(@Nonnull CharSequence self, @Nonnull String result) {
407407
if (!canBeTainted(self)) {
408408
return;
409409
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,47 @@ class StringModuleTest extends IastModuleImplTestBase {
810810
"A==>\u00cc\u00cc\u00cc<==B" | "a==>ììì<==b" | "en" | 5 | 5 | [[1, 3]]
811811
}
812812
813+
void 'onStringConstructor (#input)'() {
814+
given:
815+
final taintedObjects = ctx.getTaintedObjects()
816+
final self = addFromTaintFormat(taintedObjects, input)
817+
final result = new String(self)
818+
819+
when:
820+
module.onStringConstructor(self, result)
821+
def taintedObject = taintedObjects.get(result)
822+
823+
then:
824+
1 * tracer.activeSpan() >> span
825+
taintFormat(result, taintedObject.getRanges()) == expected
826+
827+
where:
828+
input | expected
829+
"==>123<==" | "==>123<=="
830+
sb("==>123<==") | "==>123<=="
831+
sbf("==>123<==") | "==>123<=="
832+
}
833+
834+
void 'onStringConstructor empty (#input)'() {
835+
given:
836+
final taintedObjects = ctx.getTaintedObjects()
837+
final self = addFromTaintFormat(taintedObjects, input)
838+
final result = new String(self)
839+
840+
when:
841+
module.onStringConstructor(self, result)
842+
843+
then:
844+
null == taintedObjects.get(result)
845+
result == expected
846+
847+
where:
848+
input | expected
849+
"" | ""
850+
sb("") | ""
851+
sbf("") | ""
852+
}
853+
813854
void 'test trim and make sure IastRequestContext is called'() {
814855
given:
815856
final taintedObjects = ctx.getTaintedObjects()
@@ -1269,6 +1310,10 @@ class StringModuleTest extends IastModuleImplTestBase {
12691310
return new StringBuilder(string)
12701311
}
12711312
1313+
private static StringBuilder sbf() {
1314+
return sbf('')
1315+
}
1316+
12721317
private static StringBuffer sbf(final String string) {
12731318
return new StringBuffer(string)
12741319
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,15 @@ public static String afterTrim(
201201
}
202202

203203
@CallSite.After("void java.lang.String.<init>(java.lang.String)")
204+
@CallSite.After("void java.lang.String.<init>(java.lang.StringBuffer)")
205+
@CallSite.After("void java.lang.String.<init>(java.lang.StringBuilder)")
204206
public static String afterStringConstructor(
205207
@CallSite.AllArguments @Nonnull final Object[] params,
206208
@CallSite.Return @Nonnull final String result) {
207209
final StringModule module = InstrumentationBridge.STRING;
208210
try {
209211
if (module != null) {
210-
module.onStringConstructor((String) params[0], result);
212+
module.onStringConstructor((CharSequence) params[0], result);
211213
}
212214
} catch (final Throwable e) {
213215
module.onUnexpectedException("afterStringConstructor threw", e);

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,18 @@ class StringCallSiteTest extends AgentTestRunner {
176176
InstrumentationBridge.registerIastModule(module)
177177

178178
when:
179-
final result = TestStringSuite.stringConstructor("hello")
179+
final result = TestStringSuite.stringConstructor(param)
180180

181181
then:
182-
result == 'hello'
182+
result == expected
183183
1 * module.onStringConstructor(_, _)
184184
0 * _
185+
186+
where:
187+
param | expected
188+
'hello' | 'hello'
189+
new StringBuilder('hello') | 'hello'
190+
new StringBuffer('hello') | 'hello'
185191
}
186192

187193
void 'test string format'() {

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,20 @@ public static String stringConstructor(String self) {
9292
return result;
9393
}
9494

95+
public static String stringConstructor(StringBuffer self) {
96+
LOGGER.debug("Before string stringConstructor {}", self);
97+
final String result = new String(self);
98+
LOGGER.debug("After string stringConstructor {}", result);
99+
return result;
100+
}
101+
102+
public static String stringConstructor(StringBuilder self) {
103+
LOGGER.debug("Before string stringConstructor {}", self);
104+
final String result = new String(self);
105+
LOGGER.debug("After string stringConstructor {}", result);
106+
return result;
107+
}
108+
95109
public static String stringConstructor(final byte[] value) {
96110
LOGGER.debug("Before string stringConstructor {}", value);
97111
final String result = new String(value);

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
@@ -36,7 +36,7 @@ void onStringJoin(
3636

3737
void onStringRepeat(@Nonnull String self, int count, @Nonnull String result);
3838

39-
void onStringConstructor(@Nonnull String self, @Nonnull String result);
39+
void onStringConstructor(@Nonnull CharSequence self, @Nonnull String result);
4040

4141
void onStringFormat(@Nonnull String pattern, @Nonnull Object[] params, @Nonnull String result);
4242

0 commit comments

Comments
 (0)