From 5086841b46ca2aa3dd2076b85da4f190661beb5a Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Tue, 26 Jul 2022 17:03:46 +0100 Subject: [PATCH] Kotlin: implement super-method calls If we only look at the dispatch receiver, these show up like `this` references rather than `super` references, preventing flow through super-calls. The super-interface case requires properly noting that interface methods with a body get a `default` modifier in order to avoid QL discarding the method as a possible callee. --- .../src/main/kotlin/KotlinFileExtractor.kt | 36 ++++++++++++++----- .../super-method-calls/test.expected | 2 ++ .../library-tests/super-method-calls/test.kt | 36 +++++++++++++++++++ .../library-tests/super-method-calls/test.ql | 18 ++++++++++ 4 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 java/ql/test/kotlin/library-tests/super-method-calls/test.expected create mode 100644 java/ql/test/kotlin/library-tests/super-method-calls/test.kt create mode 100644 java/ql/test/kotlin/library-tests/super-method-calls/test.ql diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt index 78a81fb0715..4100594ad35 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt @@ -888,6 +888,10 @@ open class KotlinFileExtractor( if (shortName.nameInDB != shortName.kotlinName) { tw.writeKtFunctionOriginalNames(methodId, shortName.kotlinName) } + + if (f.hasInterfaceParent() && f.body != null) { + addModifiers(id, "default") // The actual output class file may or may not have this modifier, depending on the -Xjvm-default setting. + } } tw.writeHasLocation(id, locId) @@ -1386,7 +1390,8 @@ open class KotlinFileExtractor( dispatchReceiver: IrExpression?, extensionReceiver: IrExpression?, typeArguments: List = listOf(), - extractClassTypeArguments: Boolean = false) { + extractClassTypeArguments: Boolean = false, + superQualifierSymbol: IrClassSymbol? = null) { val locId = tw.getLocation(callsite) @@ -1404,7 +1409,8 @@ open class KotlinFileExtractor( dispatchReceiver?.let { { callId -> extractExpressionExpr(dispatchReceiver, enclosingCallable, callId, -1, enclosingStmt) } }, extensionReceiver?.let { { argParent -> extractExpressionExpr(extensionReceiver, enclosingCallable, argParent, 0, enclosingStmt) } }, typeArguments, - extractClassTypeArguments + extractClassTypeArguments, + superQualifierSymbol ) } @@ -1424,7 +1430,8 @@ open class KotlinFileExtractor( extractDispatchReceiver: ((Label) -> Unit)?, extractExtensionReceiver: ((Label) -> Unit)?, typeArguments: List = listOf(), - extractClassTypeArguments: Boolean = false) { + extractClassTypeArguments: Boolean = false, + superQualifierSymbol: IrClassSymbol? = null) { val callTarget = syntacticCallTarget.target.realOverrideTarget val id = tw.getFreshIdLabel() @@ -1483,6 +1490,8 @@ open class KotlinFileExtractor( if (callTarget.shouldExtractAsStatic) { extractStaticTypeAccessQualifier(callTarget, id, locId, enclosingCallable, enclosingStmt) + } else if (superQualifierSymbol != null) { + extractSuperAccess(superQualifierSymbol.typeWith(), enclosingCallable, id, -1, enclosingStmt, locId) } else if (extractDispatchReceiver != null) { extractDispatchReceiver(id) } @@ -1744,7 +1753,7 @@ open class KotlinFileExtractor( else listOf() - extractRawMethodAccess(syntacticCallTarget, c, callable, parent, idx, enclosingStmt, (0 until c.valueArgumentsCount).map { c.getValueArgument(it) }, c.dispatchReceiver, c.extensionReceiver, typeArgs, extractClassTypeArguments) + extractRawMethodAccess(syntacticCallTarget, c, callable, parent, idx, enclosingStmt, (0 until c.valueArgumentsCount).map { c.getValueArgument(it) }, c.dispatchReceiver, c.extensionReceiver, typeArgs, extractClassTypeArguments, c.superQualifierSymbol) } fun extractSpecialEnumFunction(fnName: String){ @@ -3066,6 +3075,17 @@ open class KotlinFileExtractor( } } + private fun extractSuperAccess(irType: IrType, callable: Label, parent: Label, idx: Int, enclosingStmt: Label, locId: Label) = + tw.getFreshIdLabel().also { + val type = useType(irType) + tw.writeExprs_superaccess(it, type.javaResult.id, parent, idx) + tw.writeExprsKotlinType(it, type.kotlinResult.id) + tw.writeHasLocation(it, locId) + tw.writeCallableEnclosingExpr(it, callable) + tw.writeStatementEnclosingExpr(it, enclosingStmt) + extractTypeAccessRecursive(irType, locId, it, 0) + } + private fun extractThisAccess(e: IrGetValue, exprParent: ExprParent, callable: Label) { val containingDeclaration = declarationStack.peek() val locId = tw.getLocation(e) @@ -4020,7 +4040,7 @@ open class KotlinFileExtractor( /** * Extracts a single wildcard type access expression with no enclosing callable and statement. */ - private fun extractWildcardTypeAccess(type: TypeResults, location: Label, parent: Label, idx: Int): Label { + private fun extractWildcardTypeAccess(type: TypeResults, location: Label, parent: Label, idx: Int): Label { val id = tw.getFreshIdLabel() tw.writeExprs_wildcardtypeaccess(id, type.javaResult.id, parent, idx) tw.writeExprsKotlinType(id, type.kotlinResult.id) @@ -4031,7 +4051,7 @@ open class KotlinFileExtractor( /** * Extracts a single type access expression with no enclosing callable and statement. */ - private fun extractTypeAccess(type: TypeResults, location: Label, parent: Label, idx: Int): Label { + private fun extractTypeAccess(type: TypeResults, location: Label, parent: Label, idx: Int): Label { // TODO: elementForLocation allows us to give some sort of // location, but a proper location for the type access will // require upstream changes @@ -4057,7 +4077,7 @@ open class KotlinFileExtractor( * `extractTypeAccessRecursive` if the argument is invariant. * No enclosing callable and statement is extracted, this is useful for type access extraction in field declarations. */ - private fun extractWildcardTypeAccessRecursive(t: IrTypeArgument, location: Label, parent: Label, idx: Int) { + private fun extractWildcardTypeAccessRecursive(t: IrTypeArgument, location: Label, parent: Label, idx: Int) { val typeLabels by lazy { TypeResults(getTypeArgumentLabel(t), TypeResult(fakeKotlinType(), "TODO", "TODO")) } when (t) { is IrStarProjection -> extractWildcardTypeAccess(typeLabels, location, parent, idx) @@ -4077,7 +4097,7 @@ open class KotlinFileExtractor( * Extracts a type access expression and its child type access expressions in case of a generic type. Nested generics are also handled. * No enclosing callable and statement is extracted, this is useful for type access extraction in field declarations. */ - private fun extractTypeAccessRecursive(t: IrType, location: Label, parent: Label, idx: Int, typeContext: TypeContext = TypeContext.OTHER): Label { + private fun extractTypeAccessRecursive(t: IrType, location: Label, parent: Label, idx: Int, typeContext: TypeContext = TypeContext.OTHER): Label { val typeAccessId = extractTypeAccess(useType(t, typeContext), location, parent, idx) if (t is IrSimpleType) { t.arguments.forEachIndexed { argIdx, arg -> diff --git a/java/ql/test/kotlin/library-tests/super-method-calls/test.expected b/java/ql/test/kotlin/library-tests/super-method-calls/test.expected new file mode 100644 index 00000000000..841eb6298c7 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/super-method-calls/test.expected @@ -0,0 +1,2 @@ +| test.kt:31:17:31:24 | source(...) | test.kt:31:15:31:25 | f(...) | +| test.kt:32:17:32:24 | source(...) | test.kt:32:15:32:25 | g(...) | diff --git a/java/ql/test/kotlin/library-tests/super-method-calls/test.kt b/java/ql/test/kotlin/library-tests/super-method-calls/test.kt new file mode 100644 index 00000000000..7b3e90d7359 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/super-method-calls/test.kt @@ -0,0 +1,36 @@ +open class A { + + open fun f(x: String) = x + +} + +interface B { + + fun g(x: String) = x + +} + +interface C { + + fun g(x: String) = x + +} + +class User : A(), B, C { + + override fun f(x: String) = super.f(x) + + override fun g(x: String) = super.g(x) + + fun source() = "tainted" + + fun sink(s: String) { } + + fun test() { + + sink(this.f(source())) + sink(this.g(source())) + + } + +} diff --git a/java/ql/test/kotlin/library-tests/super-method-calls/test.ql b/java/ql/test/kotlin/library-tests/super-method-calls/test.ql new file mode 100644 index 00000000000..9a628624c91 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/super-method-calls/test.ql @@ -0,0 +1,18 @@ +import java +import semmle.code.java.dataflow.DataFlow + +class Config extends DataFlow::Configuration { + Config() { this = "abc" } + + override predicate isSource(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod().getName() = "source" + } + + override predicate isSink(DataFlow::Node n) { + n.asExpr().(Argument).getCall().getCallee().getName() = "sink" + } +} + +from Config c, DataFlow::Node n1, DataFlow::Node n2 +where c.hasFlow(n1, n2) +select n1, n2