Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IAST propagation to String valueOf #8013

Merged
merged 8 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -742,6 +742,30 @@ public String onStringReplace(
numReplacements);
}

@Override
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
public void onStringValueOf(Object param, @Nonnull String result) {
if (param == null || !canBeTainted(result)) {
return;
}
final IastContext ctx = IastContext.Provider.get();
if (ctx == null) {
return;
}
final TaintedObjects taintedObjects = ctx.getTaintedObjects();
final TaintedObject taintedParam = taintedObjects.get(param);
if (taintedParam == null) {
return;
}

final Range[] rangesParam = taintedParam.getRanges();
if (rangesParam.length == 0) {
return;
}

taintedObjects.taint(result, rangesParam);
}

/**
* Adds the tainted ranges belonging to the current parameter added via placeholder taking care of
* an optional tainted placeholder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1298,6 +1298,27 @@ class StringModuleTest extends IastModuleImplTestBase {
"==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | 0 | "==>my_o<==u==>tput<====>my_o<==u==>tput<=="
}

void 'test valueOf with (#param) and make sure IastRequestContext is called'() {
given:
final taintedObjects = ctx.getTaintedObjects()
def paramTainted = addFromTaintFormat(taintedObjects, param)
def result = String.valueOf(paramTainted)

when:
module.onStringValueOf(paramTainted, result)
def taintedObject = taintedObjects.get(result)

then:
1 * tracer.activeSpan() >> span
taintFormat(result, taintedObject.getRanges()) == expected

where:
param | expected
"==>test<==" | "==>test<=="
sb("==>test<==") | "==>test<=="
sbf("==>my_input<==") | "==>my_input<=="
smola marked this conversation as resolved.
Show resolved Hide resolved
}

private static Date date(final String pattern, final String value) {
return new SimpleDateFormat(pattern).parse(value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,4 +300,18 @@ public static String afterReplaceChar(
}
return result;
}

@CallSite.After("java.lang.String java.lang.String.valueOf(java.lang.Object)")
public static String afterValueOf(
@CallSite.Argument(0) final Object obj, @CallSite.Return final String result) {
final StringModule module = InstrumentationBridge.STRING;
if (module != null) {
try {
module.onStringValueOf(obj, result);
} catch (final Throwable e) {
module.onUnexpectedException("afterValueOf threw", e);
}
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,24 @@ class StringCallSiteTest extends AgentTestRunner {
"test" | 't' | 'T' | "TesT"
"test" | 'e' | 'E' | "tEst"
}

def 'test string valueOf call site'() {
setup:
final stringModule = Mock(StringModule)
InstrumentationBridge.registerIastModule(stringModule)

when:
final result = TestStringSuite.valueOf(input)

then:
result == expected
1 * stringModule.onStringValueOf(input, expected)
0 * _

where:
input | expected
"test" | "test"
new StringBuilder("test") | "test"
new StringBuffer("test") | "test"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,11 @@ public static String replaceFirst(
LOGGER.debug("After replace first {}", result);
return result;
}

public static String valueOf(final Object param) {
LOGGER.debug("Before valueOf {}", param);
String result = String.valueOf(param);
LOGGER.debug("After valueOf {}", result);
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ void onStringFormat(

String onStringReplace(
@Nonnull String self, String regex, String replacement, int numReplacements);

void onStringValueOf(Object param, @Nullable String result);
}
Loading