Skip to content

Commit

Permalink
Revert "Add propagation to String strip methods (#7651)" (#7664)
Browse files Browse the repository at this point in the history
This reverts commit 8da3e7a.
  • Loading branch information
smola authored and jordan-wong committed Sep 23, 2024
1 parent f63d265 commit 02e651a
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -567,43 +567,6 @@ public void onSplit(@Nonnull String self, @Nonnull String[] result) {
}
}

@Override
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
public void onStringStrip(@Nonnull String self, @Nonnull String result, boolean trailing) {
if (self == result || !canBeTainted(result)) {
return;
}
final IastContext ctx = IastContext.Provider.get();
if (ctx == null) {
return;
}
final TaintedObjects taintedObjects = ctx.getTaintedObjects();
final TaintedObject taintedSelf = taintedObjects.get(self);
if (taintedSelf == null) {
return;
}

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

int offset = 0;
if (!trailing) {
while ((offset < self.length()) && (Character.isWhitespace(self.charAt(offset)))) {
offset++;
}
}

int resultLength = result.length();

final Range[] newRanges = Ranges.forSubstring(offset, resultLength, rangesSelf);

if (newRanges != null) {
taintedObjects.taint(result, newRanges);
}
}

/**
* 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 @@ -984,62 +984,6 @@ class StringModuleTest extends IastModuleImplTestBase {
'==>testing<== the ==>test<==' | ' ' | ['==>testing<==', '==>the<==', '==>test<=='] as String[]
}
void 'test #method and make sure IastRequestContext is called'() {
given:
final taintedObjects = ctx.getTaintedObjects()
def self = addFromTaintFormat(taintedObjects, testString)
def result = self."$method"()
when:
module.onStringStrip(self, result, trailing)
def taintedObject = taintedObjects.get(result)
then:
1 * tracer.activeSpan() >> span
taintFormat(result, taintedObject.getRanges()) == expected
where:
method | trailing | testString | expected
"strip" | false | " ==>123<== " | "==>123<=="
"stripLeading" | false | " ==>123<== " | "==>123<== "
"stripTrailing" | true | " ==>123<== " | " ==>123<=="
"strip" | false | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | "==>456<== ==>ABC<=="
"stripLeading" | false | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | "==>456<== ==>ABC<== ==> <== ==> <== "
"stripTrailing" | true | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | " ==> <== ==> <== ==>456<== ==>ABC<=="
"strip" | false | " ==>123<== " | "==>123<=="
"stripLeading" | false | " ==>123<== " | "==>123<== "
"stripTrailing" | true | " ==>123<== " | " ==>123<=="
"strip" | false | "==> 123 <==" | "==>123<=="
"stripLeading" | false | "==> 123 <==" | "==>123 <=="
"stripTrailing" | true | "==> 123 <==" | "==> 123<=="
"strip" | false | " a==> b <==c " | "a==> b <==c"
"stripLeading" | false | " a==> b <==c " | "a==> b <==c "
"stripTrailing" | true | " a==> b <==c " | " a==> b <==c"
}
void 'test #method for empty string cases'() {
given:
final taintedObjects = ctx.getTaintedObjects()
def self = addFromTaintFormat(taintedObjects, testString)
def result = self."$method"()
when:
module.onStringStrip(self, result, trailing)
then:
null == taintedObjects.get(result)
result == expected
where:
method | trailing | testString | expected
"strip" | false | " ==> <== " | ""
"stripLeading" | false | " ==> <== " | ""
"stripTrailing" | true | " ==> <== " | ""
"strip" | false | "" | ""
"stripLeading" | false | "" | ""
"stripTrailing" | true | "" | ""
}
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 @@ -26,33 +26,4 @@ public static String afterRepeat(
}
return result;
}

@CallSite.After("java.lang.String java.lang.String.strip()")
@CallSite.After("java.lang.String java.lang.String.stripLeading()")
public static String afterStrip(
@CallSite.This final String self, @CallSite.Return final String result) {
final StringModule module = InstrumentationBridge.STRING;
try {
if (module != null) {
module.onStringStrip(self, result, false);
}
} catch (final Throwable e) {
module.onUnexpectedException("afterStrip threw", e);
}
return result;
}

@CallSite.After("java.lang.String java.lang.String.stripTrailing()")
public static String afterStripTrailing(
@CallSite.This final String self, @CallSite.Return final String result) {
final StringModule module = InstrumentationBridge.STRING;
try {
if (module != null) {
module.onStringStrip(self, result, true);
}
} catch (final Throwable e) {
module.onUnexpectedException("afterStripTrailing threw", e);
}
return result;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
package datadog.trace.instrumentation.java.lang.jdk11

import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.propagation.StringModule
Expand Down Expand Up @@ -33,23 +31,4 @@ class StringCallSiteTest extends AgentTestRunner {
1 * iastModule.onStringRepeat(self, count, expected)
0 * _
}

def 'test string #method call site'() {
setup:
final module = Mock(StringModule)
InstrumentationBridge.registerIastModule(module)

when:
final result = TestStringJDK11Suite."$method"(input)

then:
result == output
1 * module.onStringStrip(input, output, trailing)

where:
method | trailing | input | output
"stringStrip" | false | ' hello ' | 'hello'
"stringStripLeading" | false | ' hello ' | 'hello '
"stringStripTrailing" | true | ' hello ' | ' hello'
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,4 @@ public static String stringRepeat(String self, int count) {
LOGGER.debug("After string repeat {}", result);
return result;
}

public static String stringStrip(final String self) {
LOGGER.debug("Before string strip {}", self);
final String result = self.strip();
LOGGER.debug("After string strip {}", result);
return result;
}

public static String stringStripLeading(final String self) {
LOGGER.debug("Before string stripLeading {}", self);
final String result = self.stripLeading();
LOGGER.debug("After string stripLeading {}", result);
return result;
}

public static String stringStripTrailing(final String self) {
LOGGER.debug("Before string stripTrailing {}", self);
final String result = self.stripTrailing();
LOGGER.debug("After string stripTrailing {}", result);
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,4 @@ void onStringFormat(
@Nonnull Iterable<String> literals, @Nonnull Object[] params, @Nonnull String result);

void onSplit(final @Nonnull String self, final @Nonnull String[] result);

void onStringStrip(@Nonnull String self, @Nonnull String result, boolean trailing);
}

0 comments on commit 02e651a

Please sign in to comment.