diff --git a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie index e3ef2739905..699a84adcaf 100644 --- a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie +++ b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie @@ -256,6 +256,8 @@ 1 org.jooq.* 1 org.jruby.* 1 org.json.* +#Needed for JSON propagation +2 org.json.JSONTokener 1 org.jsoup.* 1 org.junit.* 1 org.jvnet.hk2.* diff --git a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java index e78db178697..444879d51b5 100644 --- a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java +++ b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java @@ -12,6 +12,7 @@ @CallSite(spi = IastCallSites.class) public class InputStreamReaderCallSite { + @CallSite.After("void java.io.InputStreamReader.(java.io.InputStream)") @CallSite.After( "void java.io.InputStreamReader.(java.io.InputStream, java.nio.charset.Charset)") public static InputStreamReader afterInit( diff --git a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy index 7b5a3e568ff..5dce907e0df 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy @@ -14,10 +14,17 @@ class InputStreamReaderCallSiteTest extends BaseIoCallSiteTest{ InstrumentationBridge.registerIastModule(iastModule) when: - TestInputStreamReaderSuite.init(new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()) + TestInputStreamReaderSuite.init(*args) then: 1 * iastModule.taintObjectIfTainted(_ as InputStreamReader, _ as InputStream) 0 * _ + + where: + args << [ + [new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()], + // InputStream input + [new ByteArrayInputStream("test".getBytes())]// Reader input + ] } } diff --git a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java index 57c572575a5..1d79a1a7b9b 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java +++ b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java @@ -9,4 +9,8 @@ public class TestInputStreamReaderSuite { public static InputStreamReader init(final InputStream in, Charset charset) { return new InputStreamReader(in, charset); } + + public static InputStreamReader init(final InputStream in) { + return new InputStreamReader(in); + } } diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index ac7e4b067c9..07bb471caa7 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -1,10 +1,25 @@ muzzle { pass { + name = 'all' group = 'org.json' module = 'json' versions = '[20070829, ]' assertInverse = true } + pass { + name = 'before_20241224' + group = 'org.json' + module = 'json' + versions = '[20070829, 20241224)' + assertInverse = true + } + pass { + name = 'after_20241224' + group = 'org.json' + module = 'json' + versions = '[20241224, ]' + assertInverse = true + } } apply from: "$rootDir/gradle/java.gradle" @@ -16,7 +31,7 @@ dependencies { testImplementation group: 'org.json', name: 'json', version: '20230227' testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') + testRuntimeOnly project(':dd-java-agent:instrumentation:java-io') //Needed for Reader - //FIXME: ASM - latestDepTestImplementation group: 'org.json', name: 'json', version: '20240303' + latestDepTestImplementation group: 'org.json', name: 'json', version: '+' } diff --git a/dd-java-agent/instrumentation/org-json/gradle.lockfile b/dd-java-agent/instrumentation/org-json/gradle.lockfile index 46487dd2105..098936c2c5f 100644 --- a/dd-java-agent/instrumentation/org-json/gradle.lockfile +++ b/dd-java-agent/instrumentation/org-json/gradle.lockfile @@ -116,7 +116,7 @@ org.hamcrest:hamcrest-core:1.3=latestDepTestCompileClasspath,latestDepTestRuntim org.hamcrest:hamcrest:2.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.jctools:jctools-core:3.3.0=instrumentPluginClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testRuntimeClasspath org.json:json:20230227=compileClasspath,testCompileClasspath,testRuntimeClasspath -org.json:json:20240303=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath +org.json:json:20250107=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath org.junit.jupiter:junit-jupiter-api:5.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.junit.jupiter:junit-jupiter-engine:5.9.2=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-commons:1.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 942a3905592..869de8beeb8 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -5,6 +5,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -14,8 +15,6 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; -import org.json.JSONArray; -import org.json.JSONObject; @AutoService(InstrumenterModule.class) public class JSONArrayInstrumentation extends InstrumenterModule.Iast @@ -25,6 +24,11 @@ public JSONArrayInstrumentation() { super("org-json"); } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public String instrumentedType() { return "org.json.JSONArray"; @@ -33,18 +37,11 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(String.class)), + isConstructor().and(takesArguments(0)).and(takesArgument(0, named("org.json.JSONTokener"))), getClass().getName() + "$ConstructorAdvice"); - transformer.applyAdvice( - isMethod() - .and(isPublic()) - .and(returns(Object.class)) - .and(named("get")) - .and(takesArguments(1)), - getClass().getName() + "$GetAdvice"); transformer.applyAdvice( isMethod().and(isPublic()).and(returns(Object.class)).and(named("opt")), - getClass().getName() + "$OptAdvice"); + packageName + ".OptAdvice"); } public static class ConstructorAdvice { @@ -57,44 +54,4 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } - - public static class GetAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } - - public static class OptAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java index f9a25abb04d..26e332f9956 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java @@ -19,6 +19,11 @@ public JSONCookieInstrumentation() { super("org-json"); } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public String instrumentedType() { return "org.json.Cookie"; diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java new file mode 100644 index 00000000000..5cb96ad7ffa --- /dev/null +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java @@ -0,0 +1,84 @@ +package datadog.trace.instrumentation.json; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumenterModule.class) +public class JSONObject20241224Instrumentation extends InstrumenterModule.Iast + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + public JSONObject20241224Instrumentation() { + super("org-json"); + } + + static final ElementMatcher.Junction AFTER_20241224 = + hasClassNamed("org.json.StringBuilderWriter"); + + @Override + public String muzzleDirective() { + return "after_20241224"; + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return AFTER_20241224; + } + + @Override + public String instrumentedType() { + return "org.json.JSONObject"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + // public JSONObject(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) + transformer.applyAdvice( + isConstructor() + .and(takesArguments(2)) + .and(takesArgument(0, named("org.json.JSONTokener"))) + .and(takesArgument(1, named("org.json.JSONParserConfiguration"))), + getClass().getName() + "$ConstructorAdvice"); + // private JSONObject(Map m, int recursionDepth, JSONParserConfiguration + // jsonParserConfiguration) + transformer.applyAdvice( + isConstructor() + .and(takesArguments(3)) + .and(takesArgument(0, Map.class)) + .and(takesArgument(1, int.class)) + .and(takesArgument(2, named("org.json.JSONParserConfiguration"))), + getClass().getName() + "$ConstructorAdvice"); + transformer.applyAdvice( + isMethod() + .and(isPublic()) + .and(returns(Object.class)) + .and(named("opt")) + .and(takesArguments(String.class)), + packageName + ".OptAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final Object input) { + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null && input != null) { + iastModule.taintObjectIfTainted(self, input); + } + } + } +} diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index c260d5390ad..dabee96e25c 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -1,10 +1,13 @@ package datadog.trace.instrumentation.json; +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -13,9 +16,9 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.Map; import net.bytebuddy.asm.Advice; -import org.json.JSONArray; -import org.json.JSONObject; +import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) public class JSONObjectInstrumentation extends InstrumenterModule.Iast @@ -24,6 +27,19 @@ public JSONObjectInstrumentation() { super("org-json"); } + static final ElementMatcher.Junction BEFORE_20241224 = + not(hasClassNamed("org.json.StringBuilderWriter")); + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return BEFORE_20241224; + } + + @Override + public String muzzleDirective() { + return "before_20241224"; + } + @Override public String instrumentedType() { return "org.json.JSONObject"; @@ -31,22 +47,21 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { + // public JSONObject(JSONTokener x) transformer.applyAdvice( - isConstructor().and(takesArguments(1)), getClass().getName() + "$ConstructorAdvice"); + isConstructor().and(takesArguments(1)).and(takesArgument(0, named("org.json.JSONTokener"))), + getClass().getName() + "$ConstructorAdvice"); + // private JSONObject(Map m) transformer.applyAdvice( - isMethod() - .and(isPublic()) - .and(returns(Object.class)) - .and(named("get")) - .and(takesArguments(String.class)), - getClass().getName() + "$GetAdvice"); + isConstructor().and(takesArguments(1)).and(takesArgument(0, Map.class)), + getClass().getName() + "$ConstructorAdvice"); transformer.applyAdvice( isMethod() .and(isPublic()) .and(returns(Object.class)) .and(named("opt")) .and(takesArguments(String.class)), - getClass().getName() + "$OptAdvice"); + packageName + ".OptAdvice"); } public static class ConstructorAdvice { @@ -59,44 +74,4 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } - - public static class GetAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } - - public static class OptAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java index eb51d248770..b4fb20a64c3 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java @@ -9,6 +9,7 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; +import java.io.Reader; import net.bytebuddy.asm.Advice; @AutoService(InstrumenterModule.class) @@ -24,10 +25,15 @@ public String instrumentedType() { return "org.json.JSONTokener"; } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(String.class)), + isConstructor().and(takesArguments(Reader.class)), getClass().getName() + "$ConstructorAdvice"); } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java new file mode 100644 index 00000000000..4e40cb0d098 --- /dev/null +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java @@ -0,0 +1,28 @@ +package datadog.trace.instrumentation.json; + +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.propagation.PropagationModule; +import net.bytebuddy.asm.Advice; +import org.json.JSONArray; +import org.json.JSONObject; + +public class OptAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } +} diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy index d0a97012811..e654c0f89a3 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy @@ -3,10 +3,19 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONArray import org.json.JSONObject -import org.json.JSONTokener class JSONArrayInstrumentationTest extends AgentTestRunner { + private static json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" + @Override void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") @@ -16,28 +25,17 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + final jsonObject = new JSONObject(json) + final menuObject = jsonObject.getJSONObject("menu") when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").getJSONArray("labels").get(0) + final array = menuObject.getJSONArray("labels") then: - name == "File" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) - 2 * module.taintStringIfTainted("File", _ as JSONArray) + array.length() == 2 + array.get(0) == "File" + array.get(1) == "Edit" + 1 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) 0 * _ } @@ -45,28 +43,22 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + final jsonObject = new JSONObject(json) + final jsonArray =jsonObject.getJSONObject("menu").getJSONArray("labels") when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").getJSONArray("labels").optString(0, "defaultvalue") + final name = jsonArray.optString(0, "defaultvalue") then: name == "File" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) 1 * module.taintStringIfTainted("File", _ as JSONArray) 0 * _ + + where: + method | arguments + "opt" | [0] + "optString" | [0, "defaultvalue"] + "get" | [0] + "getString" | [0, "defaultvalue"] } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy index 004b7afdd22..ca7f07eddfe 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy @@ -23,7 +23,8 @@ class JSONCookieInstrumentationTest extends AgentTestRunner { then: 1 * module.taintObjectIfTainted(_ as JSONObject, cookie) - 1 * module.taintObjectIfTainted(_ as JSONTokener, cookie) + 1 * module.taintObjectIfTainted(_ as Reader, cookie) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) 0 * _ } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy index 49be03842d2..ee0e3b897f7 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy @@ -3,6 +3,7 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONObject import org.json.JSONTokener +import spock.lang.Shared class JSONObjectInstrumentationTest extends AgentTestRunner { @@ -10,35 +11,16 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { injectSysConfig("dd.iast.enabled", "true") } - void 'test JSONObject string constructor'() { - given: - final module = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + @Shared + Map tainteds = new IdentityHashMap<>() - when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").get("name") - then: - name == "nameTest" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) - 0 * _ + void setup() { + tainteds.clear() } - void 'test JSONObject opt'() { + + void 'test JSONObject string constructor'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) @@ -53,36 +35,28 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { }}""" when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").optString("name") + new JSONObject(json) then: - name == "nameTest" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 1 * module.taintObjectIfTainted(_ as Reader, json) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) + // Two calls are necessary, once for the whole object and other for the menu object 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } - - void 'test JSONObject JSonTokenizer constructor'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) final json = '{"name": "nameTest", "value" : "valueTest"}' + final jsonTokener = new JSONTokener(json) when: - final jsonObject = new JSONObject(new JSONTokener(json)) - final name = jsonObject.get("name") + new JSONObject(jsonTokener) then: - name == "nameTest" 1 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } @@ -96,12 +70,80 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { InstrumentationBridge.registerIastModule(module) when: - final jsonObject = new JSONObject(map) - jsonObject.get("name") + new JSONObject(map) then: 1 * module.taintObjectIfTainted(_ as JSONObject, map) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } + + void 'test JSONObject #method'() { + given: + final module = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(module) + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" + final jsonObject = new JSONObject(json) + final getObject =jsonObject.getJSONObject("menu") + + when: + final name = getObject."$method"('name') + + then: + name == "nameTest" + 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) + 0 * _ + + where: + method << ['get', 'getString', 'opt', 'optString'] + } + + void 'test JSONObject elements are tainted'() { + given: + final module = Mock(PropagationModule) { + taintObjectIfTainted(_, _) >> { + if (tainteds.containsKey(it[1])) { + tainteds.put(it[0], null) + } + } + taintStringIfTainted(_, _) >> { + if (tainteds.containsKey(it[1])) { + tainteds.put(it[0], null) + } + } + } + InstrumentationBridge.registerIastModule(module) + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" + tainteds.put(json, null) + final jsonObject = new JSONObject(json) + final getObject =jsonObject.getJSONObject("menu") + + when: + final name = getObject.get('name') + final file = getObject.get('labels').get(0) + final edit = getObject.get('labels').get(1) + + then: + name == "nameTest" + tainteds.containsKey(name) + file == "File" + tainteds.containsKey(file) + edit == "Edit" + tainteds.containsKey(edit) + } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy index de8fa1da40c..806a691074a 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy @@ -5,21 +5,38 @@ import org.json.JSONTokener class JSONTokenerInstrumentationTest extends AgentTestRunner { - @Override void configurePreAgent() { + private static final String JSON_STRING = '{"name": "nameTest", "value" : "valueTest"}' // Reused JSON String + + @Override + void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") } - void 'test JSONTokener string constructor'() { + void 'test JSONTokener constructor with different argument types'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = '{"name": "nameTest", "value" : "valueTest"}' when: - new JSONTokener(json) + new JSONTokener(args) then: - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _) + if(args instanceof String) { + 1 * module.taintObjectIfTainted(_ as Reader, _ as String) + } else if (args instanceof InputStream) { + 1 * module.taintObjectIfTainted(_ as Reader, _ as InputStream) + } 0 * _ + + where: + args << [ + JSON_STRING, + // String input + new ByteArrayInputStream(JSON_STRING.bytes), + // InputStream input + new StringReader(JSON_STRING) // Reader input + ] } + }