From 5ba4f437b34f9ecc316c1f765be442f52005ae5f Mon Sep 17 00:00:00 2001 From: David Baker Effendi Date: Mon, 29 Mar 2021 21:45:56 +0200 Subject: [PATCH] :bug: Fixed Data Flow Pass Exceptions (#145) * Basic assignment CFG stream working * Things seem to be fixed * Exception throws are now handled * Merged from popthink's changes * Removed gsql jar --- CHANGELOG.md | 4 + VERSION.md | 2 +- .../kotlin/io/github/plume/oss/Extractor.kt | 19 +++- .../plume/oss/domain/model/DeltaGraph.kt | 2 + .../plume/oss/passes/graph/BaseCPGPass.kt | 90 +++++++++++-------- .../github/plume/oss/util/SootToPlumeUtil.kt | 19 ---- .../ExceptionInterproceduralTest.kt | 20 ++--- .../interprocedural/exception/Exception2.java | 5 +- 8 files changed, 92 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9e10586..177d6a00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,12 +12,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `IDriver.bulkTransaction` to replace `DeltaGraph.apply` and make database specific bulk changes. - `IdentityStmt` is now handled as part of the `LOCAL` and `IDENTIFIER` cycle - `IdentityRef` is now handled under `projectOp` +- `ThrowStmt` is now handled as a special kind of `Return` ### Fixed - External method <-`AST`- External type is now fixed. - `EVAL_TYPE` links for `MethodReturn` and `BlockVertex` on the method stubs. - `CacheOptions.cacheSize` is mutable via setters now. +- `DataFlowPass` now gets method head along with method body so the passes no longer throw exceptions. - `parseBinopExpr` had some incorrect mappings which are now fixed. ### Changed @@ -33,6 +35,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `GotoStmt` is now added as a `CONTROL_STRUCTURE_VERTEX` with `JUMP_TARGET`s removed. - CFG now connects nodes within each expression and follows the stack pointer like Joern/Ocular +Note: `StaticFieldRef` has moved from using `TypeRef` to `Identifier` otherwise data flow passes through errors. + ## [0.3.9] - 2021-03-23 ### Fixed diff --git a/VERSION.md b/VERSION.md index 4d0952d9..1d0ba9ea 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -0.3.9-SNAPSHOT +0.4.0 diff --git a/plume/src/main/kotlin/io/github/plume/oss/Extractor.kt b/plume/src/main/kotlin/io/github/plume/oss/Extractor.kt index a7117dd6..ef3ca477 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/Extractor.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/Extractor.kt @@ -47,6 +47,7 @@ import io.shiftleft.codepropertygraph.generated.NodeKeyNames.* import io.shiftleft.codepropertygraph.generated.NodeTypes.META_DATA import io.shiftleft.codepropertygraph.generated.NodeTypes.METHOD import io.shiftleft.codepropertygraph.generated.nodes.NewMetaDataBuilder +import io.shiftleft.codepropertygraph.generated.nodes.NewMethodBuilder import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -312,7 +313,17 @@ class Extractor(val driver: IDriver) { // Consumer: Receive delta graphs and write changes to the driver in serial runInsideProgressBar("Method Heads", headsToBuild.size.toLong()) { pb -> runBlocking { - repeat(headsToBuild.size) { driver.bulkTransaction(channel.receive()); pb?.step() } + repeat(headsToBuild.size) { + val dg = channel.receive() + val methodVertex = dg.changes.filterIsInstance() + .map { it.n }.filterIsInstance().first() + // Store internal method head for dataflow pass + if (!methodVertex.build().isExternal) { + PlumeStorage.methodCpgs[methodVertex.build().fullName()] = dg + } + driver.bulkTransaction(dg) + pb?.step() + } } } logger.info("All ${headsToBuild.size} method heads have been applied to the driver") @@ -410,7 +421,11 @@ class Extractor(val driver: IDriver) { val dg = BaseCPGPass(g).runPass() channel.send(dg) val (fullName, _, _) = SootToPlumeUtil.methodToStrings(g.body.method) - PlumeStorage.methodCpgs[fullName] = dg + // Combine existing method head delta with method body delta + PlumeStorage.methodCpgs[fullName] = DeltaGraph.Builder() + .addAll(PlumeStorage.methodCpgs[fullName]!!.changes) + .addAll(dg.changes) + .build() } } } diff --git a/plume/src/main/kotlin/io/github/plume/oss/domain/model/DeltaGraph.kt b/plume/src/main/kotlin/io/github/plume/oss/domain/model/DeltaGraph.kt index 5a7066d8..e9d889ad 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/domain/model/DeltaGraph.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/domain/model/DeltaGraph.kt @@ -97,6 +97,8 @@ class DeltaGraph private constructor(val changes: List) { fun deleteEdge(src: NewNodeBuilder, tgt: NewNodeBuilder, e: String) = apply { changes.add(EdgeDelete(src, tgt, e)) } + fun addAll(otherChanges: List) = apply { changes.addAll(otherChanges) } + fun build() = DeltaGraph(this) } diff --git a/plume/src/main/kotlin/io/github/plume/oss/passes/graph/BaseCPGPass.kt b/plume/src/main/kotlin/io/github/plume/oss/passes/graph/BaseCPGPass.kt index 45038921..7f3e1c34 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/passes/graph/BaseCPGPass.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/passes/graph/BaseCPGPass.kt @@ -34,7 +34,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { private var currentLine = -1 private var currentCol = -1 - private fun addToCache(e: Any, vararg ns: NewNodeBuilder, index: Int = -1) { + private fun addToStore(e: Any, vararg ns: NewNodeBuilder, index: Int = -1) { if (index == -1) methodStore.computeIfPresent(e) { _: Any, u: List -> u + ns.toList() } else methodStore.computeIfPresent(e) { _: Any, u: List -> @@ -153,11 +153,12 @@ class BaseCPGPass(private val g: BriefUnitGraph) { is AssignStmt -> projectVariableAssignment(unit, childIdx) is LookupSwitchStmt -> projectLookupSwitch(unit, childIdx) is TableSwitchStmt -> projectTableSwitch(unit, childIdx) - is InvokeStmt -> projectCallVertex(unit.invokeExpr, childIdx).apply { addToCache(unit, this, index = 0) } + is InvokeStmt -> projectCallVertex(unit.invokeExpr, childIdx).apply { addToStore(unit, this, index = 0) } is ReturnStmt -> projectReturnVertex(unit, childIdx) is ReturnVoidStmt -> projectReturnVertex(unit, childIdx) + is ThrowStmt -> projectThrowStmt(unit, childIdx) else -> { - logger.debug("Unhandled class in projectUnit ${unit.javaClass} $unit"); null + logger.debug("Unhandled class in projectUnitAsAst ${unit.javaClass} $unit"); null } } } @@ -196,6 +197,25 @@ class BaseCPGPass(private val g: BriefUnitGraph) { } } + private fun projectThrowStmt(unit: ThrowStmt, childIdx: Int): NewNodeBuilder { + val (op, _) = projectOp(unit.op, 1) + val throwVertex = NewReturnBuilder() + .order(childIdx) + .code(unit.toString()) + .lineNumber(Option.apply(unit.javaSourceStartLineNumber)) + .columnNumber(Option.apply(unit.javaSourceStartColumnNumber)) + builder.addEdge(op, throwVertex, CFG) + builder.addEdge(throwVertex, op, AST) + addToStore(unit, op, throwVertex) + PlumeStorage.getMethodStore(g.body.method) + .firstOrNull { it is NewBlockBuilder } + ?.let { block -> builder.addEdge(block, throwVertex, AST) } + PlumeStorage.getMethodStore(g.body.method) + .firstOrNull { it is NewMethodReturnBuilder } + ?.let { mtdRet -> builder.addEdge(throwVertex, mtdRet, CFG) } + return throwVertex + } + private fun projectTableSwitch(unit: TableSwitchStmt) { val switchVertices = getFromStore(unit) val switchCondition = switchVertices.first() @@ -298,7 +318,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { builder.addEdge(this, t, EVAL_TYPE) } methodLocals[head] = this - addToCache(head, this) + addToStore(head, this) } }.toList() val locals = graph.body.locals.mapIndexed { i, local: Local -> @@ -308,7 +328,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { builder.addEdge(this, t, EVAL_TYPE) } methodLocals[local] = this - addToCache(local, this) + addToStore(local, this) } }.toList() return locals + paramLocals @@ -363,15 +383,15 @@ class BaseCPGPass(private val g: BriefUnitGraph) { builder.addEdge(callVertex, expressionVertex, AST) builder.addEdge(callVertex, expressionVertex, ARGUMENT) argVertices.add(expressionVertex) - addToCache(arg, expressionVertex) + addToStore(arg, expressionVertex) } } // Save PDG arguments - addToCache(unit, *argVertices.toTypedArray()) + addToStore(unit, *argVertices.toTypedArray()) // Create the receiver for the call unit.useBoxes.filterIsInstance().firstOrNull()?.let { SootToPlumeUtil.createIdentifierVertex(it.value, currentLine, currentCol, 0).apply { - addToCache(it.value, this) + addToStore(it.value, this) builder.addEdge(callVertex, this, RECEIVER) builder.addEdge(callVertex, this, ARGUMENT) builder.addEdge(callVertex, this, AST) @@ -394,7 +414,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(currentCol)) .order(childIdx) .argumentIndex(childIdx) - addToCache(unit, switchVertex) + addToStore(unit, switchVertex) projectDefaultAndCondition(unit, switchVertex) // Handle case jumps unit.targets.forEachIndexed { i, tgt -> @@ -407,7 +427,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(tgt.javaSourceStartColumnNumber)) .order(childIdx) builder.addEdge(switchVertex, tgtV, AST) - addToCache(unit, tgtV) + addToStore(unit, tgtV) } } return switchVertex @@ -427,7 +447,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(unit.javaSourceStartColumnNumber)) .order(childIdx) .argumentIndex(childIdx) - addToCache(unit, switchVertex) + addToStore(unit, switchVertex) projectDefaultAndCondition(unit, switchVertex) // Handle case jumps for (i in 0 until unit.targetCount) { @@ -442,7 +462,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(tgt.javaSourceStartColumnNumber)) .order(childIdx) builder.addEdge(switchVertex, tgtV, AST) - addToCache(unit, tgtV) + addToStore(unit, tgtV) } } return switchVertex @@ -469,9 +489,9 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(it.javaSourceStartColumnNumber)) .order(totalTgts + 2) builder.addEdge(switchVertex, tgtV, AST) - addToCache(unit, tgtV) + addToStore(unit, tgtV) } - addToCache(unit, condition, index = 0) + addToStore(unit, condition, index = 0) } @@ -496,7 +516,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { val (conditionExpr, conditionalCfgStart) = projectBinopExpr(condition, 0) builder.addEdge(ifVertex, conditionExpr, CONDITION) builder.addEdge(ifVertex, conditionExpr, AST) - addToCache(unit, conditionalCfgStart, conditionExpr, ifVertex) + addToStore(unit, conditionalCfgStart, conditionExpr, ifVertex) return ifVertex } @@ -508,7 +528,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(unit.javaSourceStartColumnNumber)) .order(childIdx) .argumentIndex(childIdx) - addToCache(unit, gotoVertex) + addToStore(unit, gotoVertex) return gotoVertex } @@ -548,15 +568,15 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(unit.javaSourceStartColumnNumber)) val leftVert = when (leftOp) { is Local -> SootToPlumeUtil.createIdentifierVertex(leftOp, currentLine, currentCol, 1).apply { - addToCache(leftOp, this) + addToStore(leftOp, this) } is FieldRef -> projectFieldAccess(leftOp, 1) .apply { - addToCache(leftOp.field, this) + addToStore(leftOp.field, this) } is ArrayRef -> SootToPlumeUtil.createArrayRefIdentifier(leftOp, currentLine, currentCol, 1) .apply { - addToCache(leftOp.base, this) + addToStore(leftOp.base, this) } else -> { logger.debug( @@ -573,12 +593,12 @@ class BaseCPGPass(private val g: BriefUnitGraph) { }.apply { builder.addEdge(assignCall, this, AST) builder.addEdge(assignCall, this, ARGUMENT) - addToCache(leftOp, this) + addToStore(leftOp, this) } val (rightVert, rightVertCfgStart) = projectOp(rightOp, 2).apply { builder.addEdge(assignCall, this.first, AST) builder.addEdge(assignCall, this.first, ARGUMENT) - addToCache(rightOp, this.first) + addToStore(rightOp, this.first) } // This handles the CFG if the rightOp is a call or similar @@ -590,7 +610,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { builder.addEdge(rightVert, assignCall, CFG) } // Save PDG arguments - addToCache(unit, leftVert, rightVert, assignCall) + addToStore(unit, leftVert, rightVert, assignCall) return assignCall } @@ -618,17 +638,17 @@ class BaseCPGPass(private val g: BriefUnitGraph) { val (op1Vert, _) = projectOp(expr.op1, 1) builder.addEdge(binOpCall, op1Vert, AST) builder.addEdge(binOpCall, op1Vert, ARGUMENT) - addToCache(expr.op1, op1Vert) + addToStore(expr.op1, op1Vert) val (op2Vert, _) = projectOp(expr.op2, 2) builder.addEdge(binOpCall, op2Vert, AST) builder.addEdge(binOpCall, op2Vert, ARGUMENT) - addToCache(expr.op2, op2Vert) + addToStore(expr.op2, op2Vert) // Save PDG arguments builder.addEdge(op1Vert, op2Vert, CFG) builder.addEdge(op2Vert, binOpCall, CFG) - addToCache(expr, op1Vert, op2Vert, binOpCall) + addToStore(expr, op1Vert, op2Vert, binOpCall) return Pair(binOpCall, op1Vert) } @@ -651,7 +671,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { // Save PDG arguments builder.addEdge(op1, castBlock, CFG) - addToCache(expr, op1, castBlock) + addToStore(expr, op1, castBlock) return Pair(castBlock, op1) } @@ -667,7 +687,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { is InvokeExpr -> projectCallVertex(expr, childIdx) is StaticFieldRef -> projectFieldAccess(expr, childIdx) is NewExpr -> SootToPlumeUtil.createNewExpr(expr, currentLine, currentCol, childIdx) - .apply { addToCache(expr, this) } + .apply { addToStore(expr, this) } is NewArrayExpr -> createNewArrayExpr(expr, childIdx) is CaughtExceptionRef -> SootToPlumeUtil.createIdentifierVertex( expr, @@ -744,13 +764,13 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .columnNumber(Option.apply(currentCol)) .apply { fieldAccessVars.add(this) } when (fieldRef) { - is StaticFieldRef -> { // Handle Static as Type_ref? - Pair( - SootToPlumeUtil.createTypeRefVertex(fieldRef.field.declaringClass.type, currentLine, currentCol, 1), + is StaticFieldRef -> { + Pair( // TODO: Making this use an Identifier is a temporary fix for data flow passes to work + SootToPlumeUtil.createIdentifierVertex(fieldRef, currentLine, currentCol, 1), SootToPlumeUtil.createFieldIdentifierVertex(fieldRef, currentLine, currentCol, 2) ) } - is InstanceFieldRef -> { // Handle Local? and Identifier? + is InstanceFieldRef -> { Pair( SootToPlumeUtil.createIdentifierVertex(fieldRef.base, currentLine, currentCol, 1), SootToPlumeUtil.createFieldIdentifierVertex(fieldRef, currentLine, currentCol, 2) @@ -766,7 +786,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { } // Call for .fieldAccess, cast doesn't need ? // Save PDG arguments - addToCache(fieldRef, *fieldAccessVars.toTypedArray()) + addToStore(fieldRef, *fieldAccessVars.toTypedArray()) return fieldAccessBlock } @@ -778,7 +798,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .code(expr.toString()) .lineNumber(Option.apply(currentLine)) .columnNumber(Option.apply(currentCol)) - .apply { addToCache(expr, this) } + .apply { addToStore(expr, this) } private fun projectReturnVertex(ret: ReturnStmt, childIdx: Int): NewReturnBuilder { @@ -795,7 +815,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { .firstOrNull { it is NewBlockBuilder } ?.let { block -> builder.addEdge(block, retV, AST) } builder.addEdge(op1, retV, CFG) - addToCache(ret, op1, retV) + addToStore(ret, op1, retV) return retV } @@ -809,7 +829,7 @@ class BaseCPGPass(private val g: BriefUnitGraph) { PlumeStorage.getMethodStore(g.body.method) .firstOrNull { it is NewBlockBuilder } ?.let { block -> builder.addEdge(block, retV, AST) } - addToCache(ret, retV) + addToStore(ret, retV) return retV } } \ No newline at end of file diff --git a/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt b/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt index ddf47335..5ab44cde 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt @@ -146,25 +146,6 @@ object SootToPlumeUtil { .lineNumber(Option.apply(currentLine)) .columnNumber(Option.apply(currentCol)) - /** - * Creates a [NewTypeRef] from a [Value]. - */ - fun createTypeRefVertex( - type: Type, - currentLine: Int, - currentCol: Int, - childIdx: Int = 1 - ): NewTypeRefBuilder = - NewTypeRefBuilder() - .code(type.toString()) - .order(childIdx) - .argumentIndex(childIdx) - .dynamicTypeHintFullName(ListMapper.stringToScalaList(type.toQuotedString())) - .typeFullName(type.toQuotedString()) - .lineNumber(Option.apply(currentLine)) - .columnNumber(Option.apply(currentCol)) - - /** * Creates a [NewIdentifier] from a [Value]. */ diff --git a/plume/src/test/kotlin/io/github/plume/oss/extractor/interprocedural/ExceptionInterproceduralTest.kt b/plume/src/test/kotlin/io/github/plume/oss/extractor/interprocedural/ExceptionInterproceduralTest.kt index 2b27a431..30f28e46 100644 --- a/plume/src/test/kotlin/io/github/plume/oss/extractor/interprocedural/ExceptionInterproceduralTest.kt +++ b/plume/src/test/kotlin/io/github/plume/oss/extractor/interprocedural/ExceptionInterproceduralTest.kt @@ -19,7 +19,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInfo import overflowdb.Graph import java.io.File -import java.io.FileWriter import java.io.IOException class ExceptionInterproceduralTest { @@ -27,7 +26,7 @@ class ExceptionInterproceduralTest { private val driver = DriverFactory(GraphDatabase.TINKER_GRAPH) as TinkerGraphDriver private lateinit var g: Graph private var PATH: File - private val TEST_PATH = "interprocedural/exception" + private const val TEST_PATH = "interprocedural/exception" init { val testFileUrl = ExceptionInterproceduralTest::class.java.classLoader.getResource(TEST_PATH) @@ -80,19 +79,18 @@ class ExceptionInterproceduralTest { @Test fun exception2Test() { val ns = g.nodes().asSequence().toList() - val localV = ns.filterIsInstance() - + val locals = ns.filterIsInstance() val mtdV = ns.filterIsInstance().firstOrNull()?.apply { assertNotNull(this) } - assertNotNull(localV.firstOrNull { - it.name() == "e" && it.typeFullName() == "java.lang.Exception" + assertNotNull(locals.firstOrNull { + it.name() == "e1#3" && it.typeFullName() == "java.lang.Exception" }) - assertNotNull(localV.firstOrNull { it.name() == "a" && it.typeFullName() == "int" }) - assertNotNull(localV.firstOrNull { + assertNotNull(locals.firstOrNull { it.name() == "a" && it.typeFullName() == "int" }) + assertNotNull(locals.firstOrNull { it.name() == "\$stack5" && it.typeFullName() == "java.lang.Exception" }) - assertNotNull(localV.firstOrNull { it.name() == "e#3" && it.typeFullName() == "int" }) - assertNotNull(localV.firstOrNull { it.name() == "b" && it.typeFullName() == "byte" }) - assertEquals(2, g.V(mtdV!!.id()).next().out(CFG).asSequence().toList().size) + assertNotNull(locals.firstOrNull { it.name() == "e1#5" && it.typeFullName() == "int" }) + assertNotNull(locals.firstOrNull { it.name() == "b" && it.typeFullName() == "byte" }) + assertEquals(3, g.V(mtdV!!.id()).next().out(CFG).asSequence().toList().size) val parseIntCall = ns.filterIsInstance().filter { it.name() == "parseInt" } .apply { assertEquals(1, this.toList().size) }.firstOrNull().apply { assertNotNull(this) } diff --git a/plume/src/test/resources/interprocedural/exception/Exception2.java b/plume/src/test/resources/interprocedural/exception/Exception2.java index 78f48f40..8df2a53b 100644 --- a/plume/src/test/resources/interprocedural/exception/Exception2.java +++ b/plume/src/test/resources/interprocedural/exception/Exception2.java @@ -7,8 +7,11 @@ public static void main(String[] args) { int b = 2; try { a = Integer.parseInt("2"); - } catch (Exception e) { + } catch (NumberFormatException e1) { + a = 3; + } catch (Exception e2) { a = 0; + throw e2; } int c = a + b; }