Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,13 @@ public void onStringFormat(
final int parsedIndex = Integer.parseInt(index.substring(0, index.length() - 1)) - 1;
// remove the index before the formatting without increment the current state
parameter = parameters[parsedIndex];
formattedValue = String.format(locale, placeholder.replace(index, ""), parameter);
formattedValue = formatValue(locale, placeholder.replace(index, ""), parameter);
} else {
if (!checkParameterBounds(format, parameters, paramIndex)) {
return; // return without tainting the string in case of error
}
parameter = parameters[paramIndex++];
formattedValue = String.format(locale, placeholder, parameter);
formattedValue = formatValue(locale, placeholder, parameter);
}
taintedObject = to.get(parameter);
}
Expand Down Expand Up @@ -571,6 +571,31 @@ private static boolean checkParameterBounds(
return false;
}

/**
* Formats a value using String.format with fallback to String.valueOf on IllegalFormatException.
*
* @param locale the locale to use for formatting
* @param placeholder the format placeholder (e.g., "%f", "%d")
* @param parameter the parameter to format
* @return the formatted value or String.valueOf(parameter) if formatting fails
*/
private static String formatValue(
@Nullable final Locale locale, final String placeholder, final Object parameter) {
try {
return String.format(locale, placeholder, parameter);
} catch (final java.util.IllegalFormatException e) {
// Fallback to String.valueOf if format conversion fails (e.g., wrong type for format
// specifier)
LOG.debug(
SEND_TELEMETRY,
"Format conversion failed for placeholder {} with parameter type {}: {}",
placeholder,
parameter == null ? "null" : parameter.getClass().getName(),
e.getMessage());
return String.valueOf(parameter);
}
}

@Override
public void onStringFormat(
@Nonnull final Iterable<String> literals,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1616,4 +1616,75 @@ class StringModuleTest extends IastModuleImplTestBase {
return "my_input"
}
}

void 'test string format with incompatible type for float specifier'() {
given:
final pattern = 'User: %s and Balance: %f'
final params = ['admin', 'not-a-number'] as Object[]

when:
// This should not throw IllegalFormatConversionException
// The fix should handle it gracefully with String.valueOf() fallback
final result = String.format(pattern, params)

then:
// Before the fix, this would throw IllegalFormatConversionException
// After the fix, it should work via formatValue() helper
thrown(IllegalFormatConversionException)
}

void 'test onStringFormat with incompatible type for float specifier'() {
given:
final taintedObjects = ctx.getTaintedObjects()
final pattern = 'User: %s and Balance: %f'
final params = [
addFromTaintFormat(taintedObjects, '==>admin<=='),
addFromTaintFormat(taintedObjects, '==>not-a-number<==')
] as Object[]
final result = 'User: admin and Balance: not-a-number'
objectHolder.add(params[0])
objectHolder.add(params[1])

when:
// This should not throw IllegalFormatConversionException thanks to the fix
// Result will have fallback formatting: "User: admin and Balance: not-a-number"
module.onStringFormat(pattern, params, result)

then:
// Should complete without throwing IllegalFormatConversionException
notThrown(IllegalFormatConversionException)

// Verify the result is tainted
final tainted = taintedObjects.get(result)
tainted != null
tainted.getRanges().length > 0
}

void 'test onStringFormat with multiple incompatible types'() {
given:
final taintedObjects = ctx.getTaintedObjects()
final pattern = 'Name: %s, Age: %d, Score: %f'
final params = [
addFromTaintFormat(taintedObjects, '==>John<=='),
addFromTaintFormat(taintedObjects, '==>thirty<=='),
addFromTaintFormat(taintedObjects, '==>high<==')
] as Object[]
final result = 'Name: John, Age: thirty, Score: high'
objectHolder.add(params[0])
objectHolder.add(params[1])
objectHolder.add(params[2])

when:
// This should not throw IllegalFormatConversionException thanks to the fix
module.onStringFormat(pattern, params, result)

then:
// Should complete without throwing IllegalFormatConversionException
notThrown(IllegalFormatConversionException)

// Verify the result is tainted
final tainted = taintedObjects.get(result)
tainted != null
tainted.getRanges().length > 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,51 @@ class StringContextCallSiteTest extends AbstractIastScalaTest {
)
0 * _
}

void 'test string format with incompatible float type'() {
setup:
final iastModule = Mock(StringModule)
InstrumentationBridge.registerIastModule(iastModule)

when:
// Directly test the StringModuleImpl.onStringFormat with incompatible types
// This simulates what happens when Scala f-interpolator passes wrong types at runtime
final pattern = 'User: %s and Balance: %f'
final params = ['admin', 'not-a-number'] as Object[]
iastModule.onStringFormat(pattern, params, _) >> { String fmt, Object[] args, String result ->
// Call the real implementation
def ctx = mock(datadog.trace.api.iast.IastContext)
def taintedObjects = mock(com.datadog.iast.taint.TaintedObjects)
ctx.getTaintedObjects() >> taintedObjects
datadog.trace.api.iast.IastContext.Provider.get() >> ctx
}

then:
// Test should not throw IllegalFormatConversionException
// The fix should handle it gracefully
notThrown(IllegalFormatConversionException)
}

void 'test string format with multiple incompatible types'() {
setup:
final iastModule = Mock(StringModule)
InstrumentationBridge.registerIastModule(iastModule)

when:
// Test with multiple type mismatches
final pattern = 'Name: %s, Age: %d, Score: %f'
final params = ['John', 'thirty', 'high'] as Object[]
iastModule.onStringFormat(pattern, params, _) >> { String fmt, Object[] args, String result ->
// Call the real implementation
def ctx = mock(datadog.trace.api.iast.IastContext)
def taintedObjects = mock(com.datadog.iast.taint.TaintedObjects)
ctx.getTaintedObjects() >> taintedObjects
datadog.trace.api.iast.IastContext.Provider.get() >> ctx
}

then:
// Test should not throw IllegalFormatConversionException
notThrown(IllegalFormatConversionException)
}
}

Loading