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.

Sign up for GitHub

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

Link fieldAccess calls of annotated methods to original properties via member #900

Merged
merged 13 commits into from
Jan 4, 2024
Merged

Link fieldAccess calls of annotated methods to original properties via member #900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6159518
link annotated methods to the properties
karan-batavia Dec 4, 2023
3aa6837
refactor
karan-batavia Dec 4, 2023
daee627
refactor
karan-batavia Dec 4, 2023
6c0ad20
address review comments
karan-batavia Dec 5, 2023
4546061
address review comments
karan-batavia Dec 5, 2023
86455d7
addressed review comments
karan-batavia Dec 5, 2023
c9c4fa1
refactor
karan-batavia Dec 5, 2023
885f266
Merge branch 'dev' of https://github.com/Privado-Inc/privado-core int…
karan-batavia Jan 2, 2024
d26c2bf
version upgrade and referenced member link to original property
karan-batavia Jan 2, 2024
f3af6c1
test for original property link and refactoring
karan-batavia Jan 2, 2024
88eb78e
addressed review comments and fix test cases
karan-batavia Jan 2, 2024
02e247d
add replace token for yaml files
karan-batavia Jan 2, 2024
11fe54c
Merge branch 'dev' of https://github.com/Privado-Inc/privado-core int…
karan-batavia Jan 4, 2024
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
50 changes: 25 additions & 25 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,31 +42,31 @@ libraryDependencies ++= Seq(
"io.circe" %% "circe-generic" % circeVersion,
"io.circe" %% "circe-parser" % circeVersion,
// NOTE: circe-yaml currently only goes until 0.14.2 (Last checked 06/07/2023)
"io.circe" %% "circe-yaml" % circeVersion exclude ("org.yaml", "snakeyaml"),
"com.lihaoyi" %% "upickle" % Versions.upickle,
"com.lihaoyi" %% "requests" % Versions.requests,
"org.scala-lang.modules" %% "scala-xml" % "2.1.0",
"org.scala-lang.modules" %% "scala-parallel-collections" % "1.0.4",
"commons-io" % "commons-io" % "2.11.0",
"com.networknt" % "json-schema-validator" % "1.0.72",
"com.fasterxml.jackson.module" %% "jackson-module-scala" % jacksonVersion,
"com.fasterxml.jackson.dataformat" % "jackson-dataformat-yaml" % jacksonVersion exclude ("org.yaml", "snakeyaml"),
"com.github.wnameless.json" % "json-flattener" % "0.14.0",
"org.apache.logging.log4j" % "log4j-core" % "2.19.0",
"org.apache.logging.log4j" % "log4j-slf4j2-impl" % "2.19.0",
"org.apache.poi" % "poi-ooxml" % "5.2.2",
"com.github.jsqlparser" % "jsqlparser" % "4.6",
"org.apache.maven" % "maven-model" % "3.9.0",
"net.sourceforge.htmlunit" % "htmlunit" % "2.70.0",
"org.yaml" % "snakeyaml" % "1.33",
"org.scala-lang" % "scala-reflect" % "2.13.8",
"org.scala-lang" % "scala-compiler" % "2.13.8",
"com.iheart" %% "ficus" % "1.5.2" exclude ("com.typesafe", "config"),
"org.jruby" % "jruby-base" % "9.4.3.0",
"org.zeromq" % "jeromq" % "0.5.4",
"org.sangria-graphql" %% "sangria" % "4.0.0",
"com.michaelpollmeier" % "versionsort" % "1.0.11",
scalaOrganization.value %% "scala3-compiler" % scalaVersion.value
"io.circe" %% "circe-yaml" % circeVersion exclude ("org.yaml", "snakeyaml"),
"com.lihaoyi" %% "upickle" % Versions.upickle,
"com.lihaoyi" %% "requests" % Versions.requests,
"org.scala-lang.modules" %% "scala-xml" % "2.1.0",
"org.scala-lang.modules" %% "scala-parallel-collections" % "1.0.4",
"commons-io" % "commons-io" % "2.11.0",
"com.networknt" % "json-schema-validator" % "1.0.72",
"com.fasterxml.jackson.module" %% "jackson-module-scala" % jacksonVersion,
"com.fasterxml.jackson.dataformat" % "jackson-dataformat-yaml" % jacksonVersion exclude ("org.yaml", "snakeyaml"),
"com.github.wnameless.json" % "json-flattener" % "0.14.0",
"org.apache.logging.log4j" % "log4j-core" % "2.19.0",
"org.apache.logging.log4j" % "log4j-slf4j2-impl" % "2.19.0",
"org.apache.poi" % "poi-ooxml" % "5.2.2",
"com.github.jsqlparser" % "jsqlparser" % "4.6",
"org.apache.maven" % "maven-model" % "3.9.0",
"net.sourceforge.htmlunit" % "htmlunit" % "2.70.0",
"org.yaml" % "snakeyaml" % "1.33",
"org.scala-lang" % "scala-reflect" % "2.13.8",
"org.scala-lang" % "scala-compiler" % "2.13.8",
"com.iheart" %% "ficus" % "1.5.2" exclude ("com.typesafe", "config"),
"org.jruby" % "jruby-base" % "9.4.3.0",
"org.zeromq" % "jeromq" % "0.5.4",
"org.sangria-graphql" %% "sangria" % "4.0.0",
"com.michaelpollmeier" % "versionsort" % "1.0.11",
scalaOrganization.value %% "scala3-compiler" % scalaVersion.value
)

ThisBuild / Compile / scalacOptions ++= Seq("-feature", "-deprecation", "-language:implicitConversions")
Expand Down
27 changes: 17 additions & 10 deletions src/main/scala/ai/privado/languageEngine/java/passes/config/JavaPropertyLinkerPass.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ 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.
*/
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
Expand Down Expand Up @@ -70,17 +72,22 @@ 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) =>
// 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
khemrajrathore marked this conversation as resolved.
Show resolved Hide resolved
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")
Expand Down
9 changes: 6 additions & 3 deletions src/main/scala/ai/privado/passes/PropertyParserPass.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down
96 changes: 35 additions & 61 deletions src/test/scala/ai/privado/languageEngine/java/passes/config/PropertiesFilePassTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand All @@ -51,86 +52,54 @@ class AnnotationTests extends PropertiesFilePassTestBase(".properties") {
|
|class Foo {
|
|private static String loggerUrl;
|
|@Value("${slack.base.url}")
|private static final String slackWebHookURL;
|
|public AuthenticationService(UserRepository userr, SessionsR sesr, ModelMapper mapper,
| 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.l.length shouldBe 3
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"
)
}
}
}

/* 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/")

"connect the referenced member to the original property denoted by the annotated method" in {
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/")
}
}
}
Expand All @@ -157,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
}

Expand Down Expand Up @@ -272,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)
}
Expand All @@ -281,7 +249,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()

Expand Down
Loading