From 6159518316d2bd6202cb9bf41a1446822a1cea0d Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Mon, 4 Dec 2023 18:26:01 +0530 Subject: [PATCH 01/11] link annotated methods to the properties --- .../config/JavaPropertyLinkerPass.scala | 23 +++++++- .../config/PropertiesFilePassTest.scala | 54 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index 81dfd76b1..7265ae9f0 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -36,8 +36,9 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper implicit val resolver: NoResolve.type = NoResolve - override def generateParts(): Array[_ <: AnyRef] = + override def generateParts(): Array[_ <: AnyRef] = { cpg.property.iterator.filter(pair => pair.name.nonEmpty && pair.value.nonEmpty).toArray + } override def runOnPart(builder: DiffGraphBuilder, property: JavaProperty): Unit = { connectProperties(property, builder) @@ -66,11 +67,29 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper builder.addEdge(propertyNode, value, EdgeTypes.IS_USED_AT) builder.addEdge(value, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) } + + val annotatedMethodsList = annotatedMethods() + + annotatedMethodsList + .filter { case (key, _) => propertyNode.name == key.code.slice(3, key.code.length - 2) } + .foreach { case (_, value) => + builder.addEdge(propertyNode, value, EdgeTypes.IS_USED_AT) + builder.addEdge(value, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) + } } + /** List of all methods annotated with Spring's `Value` annotation + */ + private def annotatedMethods(): List[(AnnotationParameterAssign, Method)] = cpg.annotation + .fullName(".*Value.*") + .filter(_.method.nonEmpty) + .filter(_.parameterAssign.nonEmpty) + .map { x => (x.parameterAssign.next(), x.method.next()) } + .toList + /** List of all parameters annotated with Spring's `Value` annotation, along with the property name. */ - def annotatedParameters(): List[(MethodParameterIn, String)] = cpg.annotation + private def annotatedParameters(): List[(MethodParameterIn, String)] = cpg.annotation .fullName("org.springframework.*Value") .filter(_.parameter.nonEmpty) .filter(_.parameterAssign.code("\\\"\\$\\{.*\\}\\\"").nonEmpty) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index d23a0b50b..cb96d6a6e 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -94,6 +94,60 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { } } +/* Test for annotation of methods */ +class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { + override val configFileContents: String = + """ + |sample: + | url: http://www.somedomain.com/ + |""".stripMargin + + override val propertyFileContents = "" + override val codeFileContents: String = + """ + | + |import org.springframework.beans.factory.annotation.Value; + | + |class Foo { + | + |@Value("${sample.url}") + |public void setUrl( String sampleUrl ) + |{ + | String url = sampleUrl; + |} + |} + |""".stripMargin + + "ConfigFilePass" should { + "connect annotated method to property" in { + val anno: List[AstNode] = cpg.property.usedAt.l + anno.length shouldBe 1 + + anno.foreach(element => { + element.label match { + case "METHOD_PARAMETER_IN" => + val List(param: MethodParameterIn) = element.toList + param.name shouldBe "loggerBaseURL" + case "MEMBER" => + element.code shouldBe "java.lang.String slackWebHookURL" + case _ => s"Unknown label ${element.label}. Test failed" + } + }) + } + + "connect property to annotated parameter" in { + val properties = cpg.property.usedAt.originalProperty.l + properties.length shouldBe 1 + properties.foreach(prop => { + prop.name match { + case "sample.url" => prop.value shouldBe ("http://www.somedomain.com/") + case _ => s"Unknown value ${prop.value}. Test failed" + } + }) + } + } +} + class GetPropertyTests extends PropertiesFilePassTestBase(".properties") { override val configFileContents = """ |accounts.datasource.url=jdbc:mariadb://localhost:3306/accounts?useSSL=false From 3aa683744c154e6f983a04026a15cce0a2277afe Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Mon, 4 Dec 2023 18:27:57 +0530 Subject: [PATCH 02/11] refactor --- .../java/passes/config/PropertiesFilePassTest.scala | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index cb96d6a6e..8f9bd808f 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -122,20 +122,9 @@ class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { "connect annotated method to property" in { val anno: List[AstNode] = cpg.property.usedAt.l anno.length shouldBe 1 - - anno.foreach(element => { - element.label match { - case "METHOD_PARAMETER_IN" => - val List(param: MethodParameterIn) = element.toList - param.name shouldBe "loggerBaseURL" - case "MEMBER" => - element.code shouldBe "java.lang.String slackWebHookURL" - case _ => s"Unknown label ${element.label}. Test failed" - } - }) } - "connect property to annotated parameter" in { + "connect property to annotated method" in { val properties = cpg.property.usedAt.originalProperty.l properties.length shouldBe 1 properties.foreach(prop => { From daee62790ec986778863696d5c0477d3557e66d6 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Mon, 4 Dec 2023 23:01:13 +0530 Subject: [PATCH 03/11] refactor --- .../passes/config/JavaPropertyLinkerPass.scala | 14 ++++++++------ .../passes/config/PropertiesFilePassTest.scala | 14 +++++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index 7265ae9f0..7759a3da1 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -62,7 +62,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper val membersAndValues = annotatedMembers() membersAndValues - .filter { case (key, _) => propertyNode.name == key.code.slice(3, key.code.length - 2) } + .filter { case (key, _) => propertyNode.name == Option(key.code.slice(3, key.code.length - 2)).getOrElse("") } .foreach { case (_, value) => builder.addEdge(propertyNode, value, EdgeTypes.IS_USED_AT) builder.addEdge(value, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) @@ -71,7 +71,9 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper val annotatedMethodsList = annotatedMethods() annotatedMethodsList - .filter { case (key, _) => propertyNode.name == key.code.slice(3, key.code.length - 2) } + .filter { case (key, _) => + propertyNode.name == Option(key.code.slice(3, key.code.length - 2)).getOrElse("") + } .foreach { case (_, value) => builder.addEdge(propertyNode, value, EdgeTypes.IS_USED_AT) builder.addEdge(value, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) @@ -81,7 +83,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all methods annotated with Spring's `Value` annotation */ private def annotatedMethods(): List[(AnnotationParameterAssign, Method)] = cpg.annotation - .fullName(".*Value.*") + .name(".*Value.*") .filter(_.method.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.method.next()) } @@ -90,12 +92,12 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all parameters annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedParameters(): List[(MethodParameterIn, String)] = cpg.annotation - .fullName("org.springframework.*Value") + .name(".*Value.*") .filter(_.parameter.nonEmpty) .filter(_.parameterAssign.code("\\\"\\$\\{.*\\}\\\"").nonEmpty) .map { x => val literalName = x.parameterAssign.code.next() - val value = literalName.slice(3, literalName.length - 2) + val value = Option(literalName.slice(3, literalName.length - 2)).getOrElse("") (x.parameter.next(), value) } .toList @@ -103,7 +105,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all members annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedMembers(): List[(AnnotationParameterAssign, Member)] = cpg.annotation - .fullName(".*Value.*") + .name(".*Value.*") .filter(_.member.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.member.next()) } diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index 8f9bd808f..dd6e5cf4a 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -24,14 +24,14 @@ package ai.privado.languageEngine.java.passes.config import ai.privado.cache.RuleCache -import ai.privado.languageEngine.java.language._ +import ai.privado.languageEngine.java.language.* import ai.privado.model.Language import ai.privado.utility.PropertyParserPass import better.files.File import io.joern.javasrc2cpg.{Config, JavaSrc2Cpg} import io.shiftleft.codepropertygraph.generated.Cpg -import io.shiftleft.codepropertygraph.generated.nodes.{AstNode, JavaProperty, Literal, MethodParameterIn} -import io.shiftleft.semanticcpg.language._ +import io.shiftleft.codepropertygraph.generated.nodes.{AstNode, JavaProperty, Literal, Method, MethodParameterIn} +import io.shiftleft.semanticcpg.language.* import org.scalatest.BeforeAndAfterAll import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec @@ -122,6 +122,14 @@ class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { "connect annotated method to property" in { val anno: List[AstNode] = cpg.property.usedAt.l anno.length shouldBe 1 + + anno.foreach(element => { + element.label match { + case "METHOD" => + val List(methodNode: Method) = element.toList + methodNode.name shouldBe "setUrl" + } + }) } "connect property to annotated method" in { From 6c0ad209c13d493f238c51156bee3f2afa9c8152 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 5 Dec 2023 12:16:30 +0530 Subject: [PATCH 04/11] address review comments --- .../java/passes/config/JavaPropertyLinkerPass.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index 7759a3da1..4433a8557 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -83,7 +83,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all methods annotated with Spring's `Value` annotation */ private def annotatedMethods(): List[(AnnotationParameterAssign, Method)] = cpg.annotation - .name(".*Value.*") + .name("Value") .filter(_.method.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.method.next()) } @@ -92,7 +92,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all parameters annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedParameters(): List[(MethodParameterIn, String)] = cpg.annotation - .name(".*Value.*") + .name("Value") .filter(_.parameter.nonEmpty) .filter(_.parameterAssign.code("\\\"\\$\\{.*\\}\\\"").nonEmpty) .map { x => @@ -100,12 +100,15 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper val value = Option(literalName.slice(3, literalName.length - 2)).getOrElse("") (x.parameter.next(), value) } + .filter { (_, value) => + value.nonEmpty + } .toList /** List of all members annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedMembers(): List[(AnnotationParameterAssign, Member)] = cpg.annotation - .name(".*Value.*") + .name("Value") .filter(_.member.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.member.next()) } From 4546061a80329f5a7e7ce6421008b1a05f89d2de Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 5 Dec 2023 12:20:15 +0530 Subject: [PATCH 05/11] address review comments --- .../java/passes/config/JavaPropertyLinkerPass.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index 4433a8557..f64407930 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -83,7 +83,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all methods annotated with Spring's `Value` annotation */ private def annotatedMethods(): List[(AnnotationParameterAssign, Method)] = cpg.annotation - .name("Value") + .nameExact("Value") .filter(_.method.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.method.next()) } @@ -92,7 +92,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all parameters annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedParameters(): List[(MethodParameterIn, String)] = cpg.annotation - .name("Value") + .nameExact("Value") .filter(_.parameter.nonEmpty) .filter(_.parameterAssign.code("\\\"\\$\\{.*\\}\\\"").nonEmpty) .map { x => @@ -108,7 +108,7 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper /** List of all members annotated with Spring's `Value` annotation, along with the property name. */ private def annotatedMembers(): List[(AnnotationParameterAssign, Member)] = cpg.annotation - .name("Value") + .nameExact("Value") .filter(_.member.nonEmpty) .filter(_.parameterAssign.nonEmpty) .map { x => (x.parameterAssign.next(), x.member.next()) } From 86455d7f8c7030d3598cb221831fd14698202c56 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 5 Dec 2023 19:02:17 +0530 Subject: [PATCH 06/11] addressed review comments --- .../config/PropertiesFilePassTest.scala | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index dd6e5cf4a..809e15071 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -79,17 +79,11 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { } "connect property to annotated parameter" in { - val properties = cpg.property.usedAt.originalProperty.l - - properties.length shouldBe 2 - - properties.foreach(prop => { - prop.name match { - case "internal.logger.api.base" => prop.value shouldBe ("https://logger.privado.ai/") - case "slack.base.url" => prop.value shouldBe ("https://hooks.slack.com/services/some/leaking/url") - case _ => s"Unknown value ${prop.value}. Test failed" - } - }) + cpg.property.usedAt.originalProperty.l.length shouldBe 2 + cpg.property.usedAt.originalProperty.name.l shouldBe List( + "https://logger.privado.ai/", + "https://hooks.slack.com/services/some/leaking/url" + ) } } } @@ -122,7 +116,6 @@ class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { "connect annotated method to property" in { val anno: List[AstNode] = cpg.property.usedAt.l anno.length shouldBe 1 - anno.foreach(element => { element.label match { case "METHOD" => @@ -133,14 +126,8 @@ class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { } "connect property to annotated method" in { - val properties = cpg.property.usedAt.originalProperty.l - properties.length shouldBe 1 - properties.foreach(prop => { - prop.name match { - case "sample.url" => prop.value shouldBe ("http://www.somedomain.com/") - case _ => s"Unknown value ${prop.value}. Test failed" - } - }) + cpg.property.usedAt.originalProperty.l.size shouldBe 1 + cpg.property.usedAt.originalProperty.name.l shouldBe List("http://www.somedomain.com/") } } } From c9c4fa149cb85ece11496e542760d748f628707b Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 5 Dec 2023 19:06:04 +0530 Subject: [PATCH 07/11] refactor --- .../java/passes/config/PropertiesFilePassTest.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index 809e15071..929381505 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -80,7 +80,8 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { "connect property to annotated parameter" in { cpg.property.usedAt.originalProperty.l.length shouldBe 2 - cpg.property.usedAt.originalProperty.name.l shouldBe List( + cpg.property.usedAt.originalProperty.name.l shouldBe List("internal.logger.api.base", "slack.base.url") + cpg.property.usedAt.originalProperty.value.l shouldBe List( "https://logger.privado.ai/", "https://hooks.slack.com/services/some/leaking/url" ) @@ -127,7 +128,9 @@ class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { "connect property to annotated method" in { cpg.property.usedAt.originalProperty.l.size shouldBe 1 - cpg.property.usedAt.originalProperty.name.l shouldBe List("http://www.somedomain.com/") + cpg.property.usedAt.originalProperty.name.l shouldBe List("sample.url") + cpg.property.usedAt.originalProperty.value.l shouldBe List("http://www.somedomain.com/") + } } } From d26c2bf74ee4eb9f442006095d35c56652ea5630 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 2 Jan 2024 11:30:28 +0530 Subject: [PATCH 08/11] version upgrade and referenced member link to original property --- build.sbt | 2 +- .../config/JavaPropertyLinkerPass.scala | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/build.sbt b/build.sbt index 490657c16..be34daa77 100644 --- a/build.sbt +++ b/build.sbt @@ -7,7 +7,7 @@ ThisBuild / version := sys.env.getOrElse("BUILD_VERSION", "dev-SNAPSHOT") // parsed by project/Versions.scala, updated by updateDependencies.sh val cpgVersion = "1.4.33" -val joernVersion = "2.0.195" +val joernVersion = "2.0.209" val overflowdbVersion = "1.182" val requests = "0.8.0" val upickle = "3.1.2" diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index f64407930..135bfafd8 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -25,9 +25,10 @@ package ai.privado.languageEngine.java.passes.config import ai.privado.languageEngine.java.language.NodeStarters import ai.privado.tagger.PrivadoParallelCpgPass -import io.shiftleft.codepropertygraph.generated.nodes._ +import io.shiftleft.codepropertygraph.generated.nodes.* import io.shiftleft.codepropertygraph.generated.{Cpg, EdgeTypes} -import io.shiftleft.semanticcpg.language._ +import io.shiftleft.semanticcpg.language.* +import org.slf4j.LoggerFactory import overflowdb.BatchedUpdate /** This pass creates a graph layer for Java `.properties` files. @@ -35,6 +36,7 @@ import overflowdb.BatchedUpdate class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProperty](cpg) { implicit val resolver: NoResolve.type = NoResolve + val logger = LoggerFactory.getLogger(getClass) override def generateParts(): Array[_ <: AnyRef] = { cpg.property.iterator.filter(pair => pair.name.nonEmpty && pair.value.nonEmpty).toArray @@ -70,17 +72,20 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper val annotatedMethodsList = annotatedMethods() - annotatedMethodsList - .filter { case (key, _) => - propertyNode.name == Option(key.code.slice(3, key.code.length - 2)).getOrElse("") - } - .foreach { case (_, value) => - builder.addEdge(propertyNode, value, EdgeTypes.IS_USED_AT) - builder.addEdge(value, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) + annotatedMethods() + .filter { case (key, _) => propertyNode.name == Option(key.code.slice(3, key.code.length - 2)).getOrElse("") } + .foreach { case (_, method) => + val referencedMember = method.ast.fieldAccess.referencedMember.l.headOption.orNull + if (referencedMember != null) { + builder.addEdge(propertyNode, referencedMember, EdgeTypes.IS_USED_AT) + builder.addEdge(referencedMember, propertyNode, EdgeTypes.ORIGINAL_PROPERTY) + } else { + logger.debug(s"Could not find a referenced member for fieldAccess in the method ${method.name}") + } } } - /** List of all methods annotated with Spring's `Value` annotation + /** List of all methods annotated with Spring's `Value` annotation, along with the method node */ private def annotatedMethods(): List[(AnnotationParameterAssign, Method)] = cpg.annotation .nameExact("Value") From f3af6c19f653a8152c40c1dcee872696f8142396 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 2 Jan 2024 12:05:50 +0530 Subject: [PATCH 09/11] test for original property link and refactoring --- .../config/PropertiesFilePassTest.scala | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index 929381505..49160d4d7 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -29,6 +29,7 @@ import ai.privado.model.Language import ai.privado.utility.PropertyParserPass import better.files.File import io.joern.javasrc2cpg.{Config, JavaSrc2Cpg} +import io.joern.x2cpg.X2Cpg.applyDefaultOverlays import io.shiftleft.codepropertygraph.generated.Cpg import io.shiftleft.codepropertygraph.generated.nodes.{AstNode, JavaProperty, Literal, Method, MethodParameterIn} import io.shiftleft.semanticcpg.language.* @@ -51,6 +52,8 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { | |class Foo { | + |private static String loggerUrl; + | |@Value("${slack.base.url}") |private static final String slackWebHookURL; | @@ -58,34 +61,40 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { | ObjectMapper objectMapper, @Qualifier("ApiCaller") ExecutorService apiExecutor, SlackStub slackStub, | SendGridStub sgStub, @Value("${internal.logger.api.base}") String loggerBaseURL) { | } + | + |@Value("${internal.logger.api.base}") + |public void setLoggerUrl( String pLoggerUrl ) + |{ + | loggerUrl = pLoggerUrl; + |} |} |""".stripMargin "ConfigFilePass" should { "connect annotated parameter to property" in { val anno: List[AstNode] = cpg.property.usedAt.l - anno.length shouldBe 2 + anno.length shouldBe 3 - anno.foreach(element => { - element.label match { - case "METHOD_PARAMETER_IN" => - val List(param: MethodParameterIn) = element.toList - param.name shouldBe "loggerBaseURL" - case "MEMBER" => - element.code shouldBe "java.lang.String slackWebHookURL" - case _ => s"Unknown label ${element.label}. Test failed" - } - }) + anno.code.l shouldBe List( + "@Value(\"${internal.logger.api.base}\") String loggerBaseURL", + "java.lang.String loggerUrl", + "java.lang.String slackWebHookURL" + ) } "connect property to annotated parameter" in { - cpg.property.usedAt.originalProperty.l.length shouldBe 2 - cpg.property.usedAt.originalProperty.name.l shouldBe List("internal.logger.api.base", "slack.base.url") - cpg.property.usedAt.originalProperty.value.l shouldBe List( + cpg.property.usedAt.originalProperty.l.length shouldBe 3 + cpg.property.usedAt.originalProperty.name.toSet.l shouldBe List("internal.logger.api.base", "slack.base.url") + cpg.property.usedAt.originalProperty.value.toSet.l shouldBe List( "https://logger.privado.ai/", "https://hooks.slack.com/services/some/leaking/url" ) } + + "connect the referenced member to the original property denoted by the annotated method" in { + cpg.member("loggerUrl").originalProperty.l.name.head shouldBe "internal.logger.api.base" + cpg.member("loggerUrl").originalProperty.l.value.head shouldBe "https://logger.privado.ai/" + } } } @@ -281,7 +290,13 @@ abstract class PropertiesFilePassTestBase(fileExtension: String) (inputDir / "GeneralConfig.java").write(codeFileContents) val config = Config().withInputPath(inputDir.pathAsString).withOutputPath(outputFile.pathAsString) - cpg = new JavaSrc2Cpg().createCpg(config).get + cpg = new JavaSrc2Cpg() + .createCpg(config) + .map { cpg => + applyDefaultOverlays(cpg) + cpg + } + .get new PropertyParserPass(cpg, inputDir.toString(), new RuleCache, Language.JAVA).createAndApply() new JavaPropertyLinkerPass(cpg).createAndApply() From 88eb78ed7707357cae3d02751a6c1814fa77365c Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 2 Jan 2024 12:40:12 +0530 Subject: [PATCH 10/11] addressed review comments and fix test cases --- .../config/JavaPropertyLinkerPass.scala | 2 + .../config/PropertiesFilePassTest.scala | 65 ++++--------------- 2 files changed, 14 insertions(+), 53 deletions(-) diff --git a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala index 135bfafd8..9da2c0ee2 100644 --- a/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala +++ b/src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala @@ -75,6 +75,8 @@ class JavaPropertyLinkerPass(cpg: Cpg) extends PrivadoParallelCpgPass[JavaProper annotatedMethods() .filter { case (key, _) => propertyNode.name == Option(key.code.slice(3, key.code.length - 2)).getOrElse("") } .foreach { case (_, method) => + // TODO: Add support for linking multiple fieldAccess in a single method + // This will work (as expected) only if a single fieldAccess is present in the method, when not the case it will connect the referenced member of the first fieldAccess to the property node val referencedMember = method.ast.fieldAccess.referencedMember.l.headOption.orNull if (referencedMember != null) { builder.addEdge(propertyNode, referencedMember, EdgeTypes.IS_USED_AT) diff --git a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala index 49160d4d7..9dc5a6e4a 100644 --- a/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala +++ b/src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala @@ -84,62 +84,22 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") { "connect property to annotated parameter" in { cpg.property.usedAt.originalProperty.l.length shouldBe 3 - cpg.property.usedAt.originalProperty.name.toSet.l shouldBe List("internal.logger.api.base", "slack.base.url") - cpg.property.usedAt.originalProperty.value.toSet.l shouldBe List( + cpg.property.usedAt.originalProperty.name.l shouldBe List( + "internal.logger.api.base", + "internal.logger.api.base", + "slack.base.url" + ) + cpg.property.usedAt.originalProperty.value.l shouldBe List( + "https://logger.privado.ai/", "https://logger.privado.ai/", "https://hooks.slack.com/services/some/leaking/url" ) } "connect the referenced member to the original property denoted by the annotated method" in { - cpg.member("loggerUrl").originalProperty.l.name.head shouldBe "internal.logger.api.base" - cpg.member("loggerUrl").originalProperty.l.value.head shouldBe "https://logger.privado.ai/" - } - } -} - -/* Test for annotation of methods */ -class AnnotationMethodTests extends PropertiesFilePassTestBase(".yml") { - override val configFileContents: String = - """ - |sample: - | url: http://www.somedomain.com/ - |""".stripMargin - - override val propertyFileContents = "" - override val codeFileContents: String = - """ - | - |import org.springframework.beans.factory.annotation.Value; - | - |class Foo { - | - |@Value("${sample.url}") - |public void setUrl( String sampleUrl ) - |{ - | String url = sampleUrl; - |} - |} - |""".stripMargin - - "ConfigFilePass" should { - "connect annotated method to property" in { - val anno: List[AstNode] = cpg.property.usedAt.l - anno.length shouldBe 1 - anno.foreach(element => { - element.label match { - case "METHOD" => - val List(methodNode: Method) = element.toList - methodNode.name shouldBe "setUrl" - } - }) - } - - "connect property to annotated method" in { - cpg.property.usedAt.originalProperty.l.size shouldBe 1 - cpg.property.usedAt.originalProperty.name.l shouldBe List("sample.url") - cpg.property.usedAt.originalProperty.value.l shouldBe List("http://www.somedomain.com/") - + cpg.member("loggerUrl").originalProperty.size shouldBe 1 + cpg.member("loggerUrl").originalProperty.name.l shouldBe List("internal.logger.api.base") + cpg.member("loggerUrl").originalProperty.value.l shouldBe List("https://logger.privado.ai/") } } } @@ -166,8 +126,7 @@ class GetPropertyTests extends PropertiesFilePassTestBase(".properties") { "ConfigFilePass" should { "create a file node for the property file" in { - val List(_, name: String) = cpg.file.name.l - + val List(_, _, name: String) = cpg.file.name.l // The default overlays add a new file to cpg.file name.endsWith("/test.properties") shouldBe true } @@ -281,7 +240,7 @@ abstract class PropertiesFilePassTestBase(fileExtension: String) inputDir = File.newTemporaryDirectory() (inputDir / s"test$fileExtension").write(configFileContents) - (inputDir / "unrelated.file").write("foo") +// (inputDir / "unrelated.file").write("foo") if (propertyFileContents.nonEmpty) { (inputDir / "application.properties").write(propertyFileContents) } From 02e247d6a3f62cd1a9c798e49eb5a113d9e232e6 Mon Sep 17 00:00:00 2001 From: Karan Batavia Date: Tue, 2 Jan 2024 12:46:06 +0530 Subject: [PATCH 11/11] add replace token for yaml files --- .../scala/ai/privado/passes/PropertyParserPass.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/scala/ai/privado/passes/PropertyParserPass.scala b/src/main/scala/ai/privado/passes/PropertyParserPass.scala index bf6ce7516..661d34b18 100644 --- a/src/main/scala/ai/privado/passes/PropertyParserPass.scala +++ b/src/main/scala/ai/privado/passes/PropertyParserPass.scala @@ -47,8 +47,8 @@ object FileExtensions { class PropertyParserPass(cpg: Cpg, projectRoot: String, ruleCache: RuleCache, language: Language.Value) extends PrivadoParallelCpgPass[String](cpg) { - - val logger = LoggerFactory.getLogger(getClass) + val PLACEHOLDER_TOKEN_START_END = "@@" + val logger = LoggerFactory.getLogger(getClass) override def generateParts(): Array[String] = { language match { @@ -181,7 +181,10 @@ class PropertyParserPass(cpg: Cpg, projectRoot: String, ruleCache: RuleCache, la private def loadAndConvertYMLtoProperties(file: String): List[(String, String, Int)] = { try { - val yamlContent = better.files.File(file).contentAsString // Read the YAML file content as a string + val yamlContent = better.files + .File(file) + .contentAsString + .replaceAll(PLACEHOLDER_TOKEN_START_END, "") // Read the YAML file content as a string val yaml = new Yaml(new SafeConstructor(LoaderOptions())) val rootNode = yaml.compose(new StringReader(yamlContent))