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

Propagation of translateEscapes of String class #8186

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
fdda8e1
APPSEC-55380 String taint tracking: translateEscapes
sezen-datadog Jan 10, 2025
edbd4c1
TU correction
sezen-datadog Jan 10, 2025
91f4b56
non null correction
sezen-datadog Jan 10, 2025
d794020
builds
sezen-datadog Jan 10, 2025
1d28044
add module to StringCallSiteTest
sezen-datadog Jan 10, 2025
e16856d
add module to StringCallSiteTest
sezen-datadog Jan 10, 2025
fe5078c
smoke test
sezen-datadog Jan 13, 2025
33d3163
settings correction
sezen-datadog Jan 13, 2025
94fb227
test unicode added
sezen-datadog Jan 13, 2025
82e2530
Use env-entry to add tags per webapp deployment (#8138)
amarziali Jan 10, 2025
00c8cd0
fix github issue creation (#8179)
tlhunter Jan 10, 2025
542f3b8
Skip jacoco coverage for internal class (#8183)
amarziali Jan 13, 2025
19bafcb
Merge branch 'master' into sezen.leblay/APPSEC-55380-translateEscapes…
sezen-datadog Jan 13, 2025
3c388a5
smoke test spring boot for jv17
sezen-datadog Jan 13, 2025
70db9f3
whoops
sezen-datadog Jan 13, 2025
1037009
unit test
sezen-datadog Jan 13, 2025
977bff8
Merge branch 'master' into sezen.leblay/APPSEC-55380-translateEscapes…
sezen-datadog Jan 14, 2025
ca67979
unit test suppression of equal
sezen-datadog Jan 14, 2025
b8c0685
mario's idea for j17 tests
sezen-datadog Jan 14, 2025
30c46d4
mario's idea for j17 tests
sezen-datadog Jan 14, 2025
378bd84
a few more tests
sezen-datadog Jan 14, 2025
65ee3e0
Merge branch 'master' into sezen.leblay/APPSEC-55380-translateEscapes…
sezen-datadog Jan 15, 2025
13965a7
fix formatting
sezen-datadog Jan 15, 2025
4956aeb
fix formatting
sezen-datadog Jan 15, 2025
6013e2a
pr comments
sezen-datadog Jan 15, 2025
28db26f
revert StringModuleTest.groovy
sezen-datadog Jan 16, 2025
44f3db9
add ignore to string translate escapes test if version less than 15
sezen-datadog Jan 16, 2025
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 @@ -293,6 +293,27 @@ public void onStringJoin(
}
}

@Override
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
public void onStringTranslateEscapes(@Nonnull String self, @Nullable String result) {
if (!canBeTainted(result)) {
return;
}
if (self == result) { // same ref, no change in taint status
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; // original string is not tainted
}
taintedObjects.taint(result, taintedSelf.getRanges()); // only possibility left
}

@Override
@SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
public void onStringRepeat(@Nonnull String self, int count, @Nonnull String result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
import groovy.transform.CompileDynamic
import org.junit.jupiter.api.Assertions
import spock.lang.IgnoreIf

import java.text.SimpleDateFormat

Expand Down Expand Up @@ -1414,6 +1415,27 @@ class StringModuleTest extends IastModuleImplTestBase {
taintFormat(result, taintedObject.getRanges()) == "==>my_input<=="
}

@IgnoreIf({ System.getProperty('java.specification.version').toBigDecimal() < 15 })
void 'test translate escapes'() {
given:
final taintedObjects = ctx.getTaintedObjects()
def self = addFromTaintFormat(taintedObjects, testString)
def result = self.translateEscapes()

when:
module.onStringTranslateEscapes(self, result)
def taintedObject = taintedObjects.get(result)

then:
taintFormat(result, taintedObject.getRanges()) == expected

where:
testString | expected
"==>hello world\t<==" | "==>hello world\t<=="
"==>hello world\n<==" | "==>hello world\n<=="
"==>hello worldn<==" | "==>hello worldn<=="
}

void 'test valueOf with special objects and make sure IastRequestContext is called'() {
given:
final taintedObjects = ctx.getTaintedObjects()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
plugins {
id 'idea'
}

ext {
minJavaVersionForTests = JavaVersion.VERSION_15
}

apply from: "$rootDir/gradle/java.gradle"
apply plugin: 'call-site-instrumentation'

muzzle {
pass {
coreJdk()
}
}

idea {
module {
jdkName = '17'
}
}

csi {
javaVersion = JavaLanguageVersion.of(17)
}

addTestSuiteForDir('latestDepTest', 'test')

dependencies {
testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')
}

project.tasks.withType(AbstractCompile).configureEach {
setJavaVersion(it, 17)
if (it.name != 'compileCsiJava') {
sourceCompatibility = JavaVersion.VERSION_15
targetCompatibility = JavaVersion.VERSION_15
if (it instanceof JavaCompile) {
it.options.release.set(15)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package datadog.trace.instrumentation.java.lang.jdk15;

import datadog.trace.agent.tooling.csi.CallSite;
import datadog.trace.api.iast.IastCallSites;
import datadog.trace.api.iast.InstrumentationBridge;
import datadog.trace.api.iast.Propagation;
import datadog.trace.api.iast.propagation.StringModule;

@Propagation
@CallSite(
spi = IastCallSites.class,
enabled = {"datadog.trace.api.iast.IastEnabledChecks", "isMajorJavaVersionAtLeast", "15"})
public class StringCallSite {
@CallSite.After("java.lang.String java.lang.String.translateEscapes()")
public static String afterTranslateEscapes(
@CallSite.This final String self, @CallSite.Return final String result) {
final StringModule module = InstrumentationBridge.STRING;
try {
if (module != null) {
module.onStringTranslateEscapes(self, result);
}
} catch (final Throwable e) {
module.onUnexpectedException("afterTranslateEscapes threw", e);
}
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package datadog.trace.instrumentation.java.lang.jdk15

import com.github.javaparser.utils.StringEscapeUtils
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.propagation.StringModule
import foo.bar.TestStringJDK15Suite
import spock.lang.Requires

@Requires({
jvm.java15Compatible
})
class StringCallSiteTest extends AgentTestRunner {

@Override
protected void configurePreAgent() {
injectSysConfig("dd.iast.enabled", "true")
}

def 'test string translate escapes call site'() {
setup:
final iastModule = Mock(StringModule)
InstrumentationBridge.registerIastModule(iastModule)

when:
final result = TestStringJDK15Suite.stringTranslateEscapes(input)

then:
result == output
1 * iastModule.onStringTranslateEscapes(input, output)

where:
input | output
"HelloThisisaline" | "HelloThisisaline"
"Hello\tThis is a line" | "Hello"+ StringEscapeUtils.unescapeJava("\\u0009") +"This is a line"
/Hello\sThis is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0020") +"This is a line"
/Hello\"This is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0022") +"This is a line"
/Hello\0This is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0000") +"This is a line"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package foo.bar;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public abstract class TestStringJDK15Suite {

private static final Logger LOGGER = LoggerFactory.getLogger(TestStringJDK15Suite.class);

private TestStringJDK15Suite() {}

public static String stringTranslateEscapes(String self) {
LOGGER.debug("Before string translate escapes {}", self);
final String result = self.translateEscapes();
LOGGER.debug("After string translate escapes {}", result);
return result;
}
}
35 changes: 35 additions & 0 deletions dd-smoke-tests/iast-util/iast-util-17/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
plugins {
id 'idea'
id 'java-test-fixtures'
}


apply from: "$rootDir/gradle/java.gradle"

description = 'iast-smoke-tests-utils-java-17'

idea {
module {
jdkName = '17'
}
}

dependencies {
api project(':dd-smoke-tests')
compileOnly group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.2.0.RELEASE'

testFixturesImplementation testFixtures(project(":dd-smoke-tests:iast-util"))
}

project.tasks.withType(AbstractCompile).configureEach {
setJavaVersion(it, 17)
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
if (it instanceof JavaCompile) {
it.options.release.set(17)
}
}

forbiddenApisMain {
failOnMissingClasses = false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package datadog.smoketest.springboot.controller;

import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("/string")
public class StringOperationController {

@PostMapping("/translateEscapes")
public String translateEscapes(@RequestParam(value = "parameter") final String parameter) {
parameter.translateEscapes();
return "ok";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package datadog.smoketest

import com.github.javaparser.utils.StringEscapeUtils
import okhttp3.FormBody
import okhttp3.Request

import static datadog.trace.api.config.IastConfig.IAST_DEBUG_ENABLED
import static datadog.trace.api.config.IastConfig.IAST_DETECTION_MODE
import static datadog.trace.api.config.IastConfig.IAST_ENABLED

abstract class AbstractIast17SpringBootTest extends AbstractIastServerSmokeTest {

@Override
ProcessBuilder createProcessBuilder() {
String springBootShadowJar = System.getProperty('datadog.smoketest.springboot.shadowJar.path')

List<String> command = []
command.add(javaPath())
command.addAll(defaultJavaProperties)
command.addAll(iastJvmOpts())
command.addAll((String[]) ['-jar', springBootShadowJar, "--server.port=${httpPort}"])
ProcessBuilder processBuilder = new ProcessBuilder(command)
processBuilder.directory(new File(buildDirectory))
// Spring will print all environment variables to the log, which may pollute it and affect log assertions.
processBuilder.environment().clear()
return processBuilder
}

protected List<String> iastJvmOpts() {
return [
withSystemProperty(IAST_ENABLED, true),
withSystemProperty(IAST_DETECTION_MODE, 'FULL'),
withSystemProperty(IAST_DEBUG_ENABLED, true),
]
}

void 'test String translateEscapes'() {
setup:
final url = "http://localhost:${httpPort}/string/translateEscapes"
final body = new FormBody.Builder()
.add('parameter', value)
.build()
final request = new Request.Builder().url(url).post(body).build()


when:
client.newCall(request).execute()

then:
hasTainted { tainted ->
tainted.value == expected
}

where:
value | expected
"withEscape\ttab" | "withEscape" + Character.toString((char)9) + "tab"
"withEscape\nnewline" | "withEscape" + StringEscapeUtils.unescapeJava("\\u000A")+ "newline"
"withEscape\bbackline" | "withEscape" + StringEscapeUtils.unescapeJava("\\u0008")+ "backline"
}
}
45 changes: 45 additions & 0 deletions dd-smoke-tests/springboot-java-17/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
plugins {
id 'java'
id 'org.springframework.boot' version '2.7.15'
id 'io.spring.dependency-management' version '1.0.15.RELEASE'
id 'java-test-fixtures'
}

ext {
minJavaVersionForTests = JavaVersion.VERSION_17
}

apply from: "$rootDir/gradle/java.gradle"
description = 'SpringBoot Java 17 Smoke Tests.'

repositories {
mavenCentral()
}

dependencies {
implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.2.0.RELEASE'

testImplementation project(':dd-smoke-tests')
testImplementation testFixtures(project(":dd-smoke-tests:iast-util:iast-util-17"))
testImplementation testFixtures(project(':dd-smoke-tests:iast-util'))

implementation project(':dd-smoke-tests:iast-util:iast-util-17')
}

project.tasks.withType(AbstractCompile).configureEach {
setJavaVersion(it, 17)
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
if (it instanceof JavaCompile) {
it.options.release.set(17)
}
}

forbiddenApisMain {
failOnMissingClasses = false
}

tasks.withType(Test).configureEach {
dependsOn "bootJar"
jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.bootJar.archiveFile.get()}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package datadog.smoketest.springboot;

import java.lang.management.ManagementFactory;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class SpringbootApplication {

public static void main(final String[] args) {
SpringApplication.run(SpringbootApplication.class, args);
System.out.println("Started in " + ManagementFactory.getRuntimeMXBean().getUptime() + "ms");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package datadog.smoketest.springboot

import datadog.smoketest.AbstractIast17SpringBootTest

class IastSpringBootSmokeTest extends AbstractIast17SpringBootTest {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need empty test class?

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ void onStringJoin(

void onStringToUpperCase(@Nonnull String self, @Nullable String result);

void onStringTranslateEscapes(@Nonnull String self, @Nullable String result);

void onStringToLowerCase(@Nonnull String self, @Nullable String result);

void onStringTrim(@Nonnull String self, @Nullable String result);
Expand Down
Loading
Loading