Skip to content

Commit 21404c9

Browse files
committed
Fix
1 parent 59d7f4e commit 21404c9

File tree

3 files changed

+144
-2
lines changed

3 files changed

+144
-2
lines changed

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,13 +528,13 @@ public void onStringFormat(
528528
final int parsedIndex = Integer.parseInt(index.substring(0, index.length() - 1)) - 1;
529529
// remove the index before the formatting without increment the current state
530530
parameter = parameters[parsedIndex];
531-
formattedValue = String.format(locale, placeholder.replace(index, ""), parameter);
531+
formattedValue = formatValue(locale, placeholder.replace(index, ""), parameter);
532532
} else {
533533
if (!checkParameterBounds(format, parameters, paramIndex)) {
534534
return; // return without tainting the string in case of error
535535
}
536536
parameter = parameters[paramIndex++];
537-
formattedValue = String.format(locale, placeholder, parameter);
537+
formattedValue = formatValue(locale, placeholder, parameter);
538538
}
539539
taintedObject = to.get(parameter);
540540
}
@@ -571,6 +571,31 @@ private static boolean checkParameterBounds(
571571
return false;
572572
}
573573

574+
/**
575+
* Formats a value using String.format with fallback to String.valueOf on IllegalFormatException.
576+
*
577+
* @param locale the locale to use for formatting
578+
* @param placeholder the format placeholder (e.g., "%f", "%d")
579+
* @param parameter the parameter to format
580+
* @return the formatted value or String.valueOf(parameter) if formatting fails
581+
*/
582+
private static String formatValue(
583+
@Nullable final Locale locale, final String placeholder, final Object parameter) {
584+
try {
585+
return String.format(locale, placeholder, parameter);
586+
} catch (final java.util.IllegalFormatException e) {
587+
// Fallback to String.valueOf if format conversion fails (e.g., wrong type for format
588+
// specifier)
589+
LOG.debug(
590+
SEND_TELEMETRY,
591+
"Format conversion failed for placeholder {} with parameter type {}: {}",
592+
placeholder,
593+
parameter == null ? "null" : parameter.getClass().getName(),
594+
e.getMessage());
595+
return String.valueOf(parameter);
596+
}
597+
}
598+
574599
@Override
575600
public void onStringFormat(
576601
@Nonnull final Iterable<String> literals,

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,4 +1616,75 @@ class StringModuleTest extends IastModuleImplTestBase {
16161616
return "my_input"
16171617
}
16181618
}
1619+
1620+
void 'test string format with incompatible type for float specifier'() {
1621+
given:
1622+
final pattern = 'User: %s and Balance: %f'
1623+
final params = ['admin', 'not-a-number'] as Object[]
1624+
1625+
when:
1626+
// This should not throw IllegalFormatConversionException
1627+
// The fix should handle it gracefully with String.valueOf() fallback
1628+
final result = String.format(pattern, params)
1629+
1630+
then:
1631+
// Before the fix, this would throw IllegalFormatConversionException
1632+
// After the fix, it should work via formatValue() helper
1633+
thrown(IllegalFormatConversionException)
1634+
}
1635+
1636+
void 'test onStringFormat with incompatible type for float specifier'() {
1637+
given:
1638+
final taintedObjects = ctx.getTaintedObjects()
1639+
final pattern = 'User: %s and Balance: %f'
1640+
final params = [
1641+
addFromTaintFormat(taintedObjects, '==>admin<=='),
1642+
addFromTaintFormat(taintedObjects, '==>not-a-number<==')
1643+
] as Object[]
1644+
final result = 'User: admin and Balance: not-a-number'
1645+
objectHolder.add(params[0])
1646+
objectHolder.add(params[1])
1647+
1648+
when:
1649+
// This should not throw IllegalFormatConversionException thanks to the fix
1650+
// Result will have fallback formatting: "User: admin and Balance: not-a-number"
1651+
module.onStringFormat(pattern, params, result)
1652+
1653+
then:
1654+
// Should complete without throwing IllegalFormatConversionException
1655+
notThrown(IllegalFormatConversionException)
1656+
1657+
// Verify the result is tainted
1658+
final tainted = taintedObjects.get(result)
1659+
tainted != null
1660+
tainted.getRanges().length > 0
1661+
}
1662+
1663+
void 'test onStringFormat with multiple incompatible types'() {
1664+
given:
1665+
final taintedObjects = ctx.getTaintedObjects()
1666+
final pattern = 'Name: %s, Age: %d, Score: %f'
1667+
final params = [
1668+
addFromTaintFormat(taintedObjects, '==>John<=='),
1669+
addFromTaintFormat(taintedObjects, '==>thirty<=='),
1670+
addFromTaintFormat(taintedObjects, '==>high<==')
1671+
] as Object[]
1672+
final result = 'Name: John, Age: thirty, Score: high'
1673+
objectHolder.add(params[0])
1674+
objectHolder.add(params[1])
1675+
objectHolder.add(params[2])
1676+
1677+
when:
1678+
// This should not throw IllegalFormatConversionException thanks to the fix
1679+
module.onStringFormat(pattern, params, result)
1680+
1681+
then:
1682+
// Should complete without throwing IllegalFormatConversionException
1683+
notThrown(IllegalFormatConversionException)
1684+
1685+
// Verify the result is tainted
1686+
final tainted = taintedObjects.get(result)
1687+
tainted != null
1688+
tainted.getRanges().length > 0
1689+
}
16191690
}

dd-java-agent/instrumentation/scala/src/test/groovy/datadog/trace/instrumentation/scala/StringContextCallSiteTest.groovy

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,5 +99,51 @@ class StringContextCallSiteTest extends AbstractIastScalaTest {
9999
)
100100
0 * _
101101
}
102+
103+
void 'test string format with incompatible float type'() {
104+
setup:
105+
final iastModule = Mock(StringModule)
106+
InstrumentationBridge.registerIastModule(iastModule)
107+
108+
when:
109+
// Directly test the StringModuleImpl.onStringFormat with incompatible types
110+
// This simulates what happens when Scala f-interpolator passes wrong types at runtime
111+
final pattern = 'User: %s and Balance: %f'
112+
final params = ['admin', 'not-a-number'] as Object[]
113+
iastModule.onStringFormat(pattern, params, _) >> { String fmt, Object[] args, String result ->
114+
// Call the real implementation
115+
def ctx = mock(datadog.trace.api.iast.IastContext)
116+
def taintedObjects = mock(com.datadog.iast.taint.TaintedObjects)
117+
ctx.getTaintedObjects() >> taintedObjects
118+
datadog.trace.api.iast.IastContext.Provider.get() >> ctx
119+
}
120+
121+
then:
122+
// Test should not throw IllegalFormatConversionException
123+
// The fix should handle it gracefully
124+
notThrown(IllegalFormatConversionException)
125+
}
126+
127+
void 'test string format with multiple incompatible types'() {
128+
setup:
129+
final iastModule = Mock(StringModule)
130+
InstrumentationBridge.registerIastModule(iastModule)
131+
132+
when:
133+
// Test with multiple type mismatches
134+
final pattern = 'Name: %s, Age: %d, Score: %f'
135+
final params = ['John', 'thirty', 'high'] as Object[]
136+
iastModule.onStringFormat(pattern, params, _) >> { String fmt, Object[] args, String result ->
137+
// Call the real implementation
138+
def ctx = mock(datadog.trace.api.iast.IastContext)
139+
def taintedObjects = mock(com.datadog.iast.taint.TaintedObjects)
140+
ctx.getTaintedObjects() >> taintedObjects
141+
datadog.trace.api.iast.IastContext.Provider.get() >> ctx
142+
}
143+
144+
then:
145+
// Test should not throw IllegalFormatConversionException
146+
notThrown(IllegalFormatConversionException)
147+
}
102148
}
103149

0 commit comments

Comments
 (0)