Skip to content

Commit

Permalink
[c#] Safe Download Dependency Handling & Cleanups (#4367)
Browse files Browse the repository at this point in the history
* Fixed non-exhaustive matching on string interpolation handling
* Wrapped the package version check in a try-catch
* Ignore looking to nuget for packages that are internal to a potential c# monolith
* Fixed test config loading in fixture
  • Loading branch information
DavidBakerEffendi authored Mar 19, 2024
1 parent 08271e2 commit 9001c04
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import io.shiftleft.passes.CpgPassBase
import org.slf4j.LoggerFactory

import java.nio.file.Paths
import scala.collection.mutable
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.Duration
import scala.concurrent.{Await, Future}
Expand All @@ -45,10 +46,12 @@ class CSharpSrc2Cpg extends X2CpgFrontend[Config] {

val hash = HashUtil.sha256(astCreators.map(_.parserResult).map(x => Paths.get(x.fullPath)))
new MetaDataPass(cpg, Languages.CSHARPSRC, config.inputPath, Option(hash)).createAndApply()
new DependencyPass(cpg, buildFiles(config)).createAndApply()

val packageIds = mutable.HashSet.empty[String]
new DependencyPass(cpg, buildFiles(config), packageIds.add).createAndApply()
// If "download dependencies" is enabled, then fetch dependencies and resolve their symbols for additional types
val programSummary = if (config.downloadDependencies) {
DependencyDownloader(cpg, config, internalProgramSummary).download()
DependencyDownloader(cpg, config, internalProgramSummary, packageIds.toSet).download()
} else {
internalProgramSummary
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,9 +445,11 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
.arr
.map(createDotNetNodeInfo)
.flatMap { expr =>
expr.node match
expr.node match {
case InterpolatedStringText => astForInterpolatedStringText(expr)
case Interpolation => astForInterpolation(expr)
case _ => Nil
}
}
.toSeq

Expand All @@ -456,13 +458,17 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
.json(ParserKeys.Contents)
.arr
.map(createDotNetNodeInfo)
.map { node =>
node.node match
.flatMap { node =>
node.node match {
case InterpolatedStringText =>
node
.json(ParserKeys.TextToken)(ParserKeys.Value)
.str // Accessing node.json directly because DotNetNodeInfo contains stripped code, and does not contain braces
case Interpolation => node.json(ParserKeys.MetaData)(ParserKeys.Code).str
Try(
node
.json(ParserKeys.TextToken)(ParserKeys.Value)
.str
).toOption // Accessing node.json directly because DotNetNodeInfo contains stripped code, and does not contain braces
case Interpolation => Try(node.json(ParserKeys.MetaData)(ParserKeys.Code).str).toOption
case _ => None
}
}
.mkString("")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import org.slf4j.LoggerFactory

import scala.util.{Failure, Try}

class DependencyPass(cpg: Cpg, buildFiles: List[String]) extends ForkJoinParallelCpgPass[File](cpg) {
class DependencyPass(cpg: Cpg, buildFiles: List[String], registerPackageId: String => _)
extends ForkJoinParallelCpgPass[File](cpg) {

private val logger = LoggerFactory.getLogger(getClass)

Expand All @@ -18,6 +19,14 @@ class DependencyPass(cpg: Cpg, buildFiles: List[String]) extends ForkJoinParalle
override def runOnPart(builder: DiffGraphBuilder, part: File): Unit = {
SecureXmlParsing.parseXml(part.contentAsString) match {
case Some(xml) if xml.label == "Project" =>
// Find packageId (useful for monoliths)
xml.child
.collect { case x if x.label == "PropertyGroup" => x.child }
.flatten
.collect {
case packageId if packageId.label == "PackageId" => registerPackageId(packageId.text)
}
// Register dependencies
xml.child
.collect { case x if x.label == "ItemGroup" => x.child }
.flatten
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ import scala.util.{Failure, Success, Try, Using}
* @see
* <a href="https://learn.microsoft.com/en-us/nuget/api/overview">NuGet API</a>
*/
class DependencyDownloader(cpg: Cpg, config: Config, internalProgramSummary: CSharpProgramSummary) {
class DependencyDownloader(
cpg: Cpg,
config: Config,
internalProgramSummary: CSharpProgramSummary,
internalPackages: Set[String] = Set.empty
) {

private val logger = LoggerFactory.getLogger(getClass)

Expand All @@ -49,8 +54,8 @@ class DependencyDownloader(cpg: Cpg, config: Config, internalProgramSummary: CSh
* true if the dependency is already in the given summary, false if otherwise.
*/
private def isAlreadySummarized(dependency: Dependency): Boolean = {
// TODO: Implement
false
// TODO: Check internalSummaries too
internalPackages.contains(dependency.name)
}

private case class NuGetPackageVersions(versions: List[String]) derives ReadWriter
Expand All @@ -67,12 +72,17 @@ class DependencyDownloader(cpg: Cpg, config: Config, internalProgramSummary: CSh
*/
private def downloadDependency(targetDir: File, dependency: Dependency): Unit = {

def getVersion(packageName: String): Option[String] = {
def getVersion(packageName: String): Option[String] = Try {
Using.resource(URI(s"https://$NUGET_BASE_API_V3/${packageName.toLowerCase}/index.json").toURL.openStream()) {
is =>
Try(read[NuGetPackageVersions](ujson.Readable.fromByteArray(is.readAllBytes()))).toOption
.flatMap(_.versions.lastOption)
}
} match {
case Failure(_) =>
logger.error(s"Unable to resolve `index.json` for `$packageName`, skipping...`")
None
case Success(x) => x
}

def createUrl(packageType: String, version: String): URL = {
Expand Down Expand Up @@ -168,12 +178,14 @@ class DependencyDownloader(cpg: Cpg, config: Config, internalProgramSummary: CSh

// Move and merge files
val libDir = targetDir / "lib"
// Sometimes these dependencies will include DLLs for multiple version of dotnet, we only want one
libDir.listRecursively.filterNot(_.isDirectory).distinctBy(_.name).foreach { f =>
f.copyTo(targetDir / f.name)
if (libDir.isDirectory) {
// Sometimes these dependencies will include DLLs for multiple version of dotnet, we only want one
libDir.listRecursively.filterNot(_.isDirectory).distinctBy(_.name).foreach { f =>
f.copyTo(targetDir / f.name)
}
// Clean-up lib dir
libDir.delete(swallowIOExceptions = true)
}
// Clean-up lib dir
libDir.delete(swallowIOExceptions = true)
}

/** Given a directory of all the summaries, will produce a summary thereof.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.joern.csharpsrc2cpg.querying.ast

import io.joern.csharpsrc2cpg.testfixtures.CSharpCode2CpgFixture
import io.joern.csharpsrc2cpg.Config
import io.shiftleft.semanticcpg.language.*

class DependencyTests extends CSharpCode2CpgFixture {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ class DefaultTestCpgWithCSharp extends DefaultTestCpg with CSharpFrontend with S
}

override def applyPostProcessingPasses(): Unit = {
CSharpSrc2Cpg.postProcessingPasses(this, config).foreach(_.createAndApply())
CSharpSrc2Cpg
.postProcessingPasses(this, getConfig().map(_.asInstanceOf[Config]).getOrElse(defaultConfig))
.foreach(_.createAndApply())
super.applyPostProcessingPasses()
}

Expand All @@ -73,7 +75,7 @@ trait CSharpFrontend extends LanguageFrontend {

override val fileSuffix: String = ".cs"

implicit val config: Config =
implicit lazy val defaultConfig: Config =
getConfig()
.map(_.asInstanceOf[Config])
.getOrElse(Config().withSchemaValidation(ValidationMode.Enabled))
Expand Down

0 comments on commit 9001c04

Please sign in to comment.