diff --git a/java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt b/java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt index 43cfad2f621c..0a9e8df80730 100644 --- a/java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt @@ -1,5 +1,6 @@ package com.github.codeql +import com.github.codeql.utils.ClassInstanceStack import com.github.codeql.utils.isExternalFileClassMember import com.semmle.extractor.java.OdasaOutput import com.semmle.util.data.StringDigestor @@ -18,6 +19,7 @@ class ExternalDeclExtractor( val compression: Compression, val invocationTrapFile: String, val sourceFilePath: String, + val classInstanceStack: ClassInstanceStack, val primitiveTypeMapping: PrimitiveTypeMapping, val pluginContext: IrPluginContext, val globalExtensionState: KotlinExtractorGlobalState, @@ -163,6 +165,7 @@ class ExternalDeclExtractor( binaryPath, manager, this, + classInstanceStack, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt b/java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt index 1fc8ee37fc03..9f524de737e2 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt @@ -1,5 +1,6 @@ package com.github.codeql +import com.github.codeql.utils.ClassInstanceStack import com.github.codeql.utils.versions.usesK2 import com.semmle.util.files.FileUtil import com.semmle.util.trap.pathtransformers.PathTransformer @@ -151,6 +152,7 @@ class KotlinExtractorExtension( } val compression = getCompression(logger) + val classInstanceStack = ClassInstanceStack() val primitiveTypeMapping = PrimitiveTypeMapping(logger, pluginContext) // FIXME: FileUtil expects a static global logger // which should be provided by SLF4J's factory facility. For now we set it here. @@ -182,6 +184,7 @@ class KotlinExtractorExtension( trapDir, srcDir, file, + classInstanceStack, primitiveTypeMapping, pluginContext, globalExtensionState @@ -358,6 +361,7 @@ private fun doFile( dbTrapDir: File, dbSrcDir: File, srcFile: IrFile, + classInstanceStack: ClassInstanceStack, primitiveTypeMapping: PrimitiveTypeMapping, pluginContext: IrPluginContext, globalExtensionState: KotlinExtractorGlobalState @@ -415,6 +419,7 @@ private fun doFile( compression, invocationTrapFile, srcFilePath, + classInstanceStack, primitiveTypeMapping, pluginContext, globalExtensionState, @@ -429,6 +434,7 @@ private fun doFile( srcFilePath, null, externalDeclExtractor, + classInstanceStack, primitiveTypeMapping, pluginContext, KotlinFileExtractor.DeclarationStack(), diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt index 376736611d1c..72c766bb0828 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt @@ -62,6 +62,7 @@ open class KotlinFileExtractor( val filePath: String, dependencyCollector: OdasaOutput.TrapFileManager?, externalClassExtractor: ExternalDeclExtractor, + classInstanceStack: ClassInstanceStack, primitiveTypeMapping: PrimitiveTypeMapping, pluginContext: IrPluginContext, val declarationStack: DeclarationStack, @@ -72,6 +73,7 @@ open class KotlinFileExtractor( tw, dependencyCollector, externalClassExtractor, + classInstanceStack, primitiveTypeMapping, pluginContext, globalExtensionState @@ -496,12 +498,17 @@ open class KotlinFileExtractor( } extractClassModifiers(c, id) - extractClassSupertypes( - c, - id, - if (argsIncludingOuterClasses == null) ExtractSupertypesMode.Raw - else ExtractSupertypesMode.Specialised(argsIncludingOuterClasses) - ) + classInstanceStack.push(c) + try { + extractClassSupertypes( + c, + id, + if (argsIncludingOuterClasses == null) ExtractSupertypesMode.Raw + else ExtractSupertypesMode.Specialised(argsIncludingOuterClasses) + ) + } finally { + classInstanceStack.pop() + } val locId = getLocation(c, argsIncludingOuterClasses) tw.writeHasLocation(id, locId) diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt index 2f87c77f8ee1..83cbec771fc5 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt @@ -49,6 +49,7 @@ open class KotlinUsesExtractor( open val tw: TrapWriter, val dependencyCollector: OdasaOutput.TrapFileManager?, val externalClassExtractor: ExternalDeclExtractor, + val classInstanceStack: ClassInstanceStack, val primitiveTypeMapping: PrimitiveTypeMapping, val pluginContext: IrPluginContext, val globalExtensionState: KotlinExtractorGlobalState @@ -182,6 +183,7 @@ open class KotlinUsesExtractor( filePath, dependencyCollector, externalClassExtractor, + classInstanceStack, primitiveTypeMapping, pluginContext, newDeclarationStack, @@ -199,6 +201,7 @@ open class KotlinUsesExtractor( clsFile.path, dependencyCollector, externalClassExtractor, + classInstanceStack, primitiveTypeMapping, pluginContext, newDeclarationStack, @@ -537,6 +540,19 @@ open class KotlinUsesExtractor( return Pair(p?.first ?: c, p?.second ?: argsIncludingOuterClassesBeforeReplacement) } + private fun avoidInfiniteRecursion( + pair: Pair?> + ): Pair?> { + val c = pair.first + val args = pair.second + if (args != null && args.isNotEmpty() && classInstanceStack.possiblyCyclicExtraction(c, args)) { + logger.warn("Making use of ${c.name} a raw type to avoid infinite recursion") + return Pair(c, null) + } else { + return pair + } + } + // `typeArgs` can be null to describe a raw generic type. // For non-generic types it will be zero-length list. private fun addClassLabel( @@ -545,7 +561,7 @@ open class KotlinUsesExtractor( inReceiverContext: Boolean = false ): TypeResult { val replaced = - tryReplaceType(cBeforeReplacement, argsIncludingOuterClassesBeforeReplacement) + avoidInfiniteRecursion(tryReplaceType(cBeforeReplacement, argsIncludingOuterClassesBeforeReplacement)) val replacedClass = replaced.first val replacedArgsIncludingOuterClasses = replaced.second diff --git a/java/kotlin-extractor/src/main/kotlin/utils/ClassInstanceStack.kt b/java/kotlin-extractor/src/main/kotlin/utils/ClassInstanceStack.kt new file mode 100644 index 000000000000..fdd0deb5151f --- /dev/null +++ b/java/kotlin-extractor/src/main/kotlin/utils/ClassInstanceStack.kt @@ -0,0 +1,47 @@ +package com.github.codeql.utils + +import java.util.Stack +import org.jetbrains.kotlin.ir.declarations.IrClass +import org.jetbrains.kotlin.ir.symbols.IrClassSymbol +import org.jetbrains.kotlin.ir.types.* + +class ClassInstanceStack { + private val stack: Stack = Stack() + + fun push(c: IrClass) = stack.push(c) + fun pop() = stack.pop() + + private fun checkTypeArgs(sym: IrClassSymbol, args: List): Boolean { + for (arg in args) { + if (arg is IrTypeProjection) { + if (checkType(sym, arg.type)) { + return true + } + } + } + return false + } + + private fun checkType(sym: IrClassSymbol, type: IrType): Boolean { + if (type is IrSimpleType) { + val decl = type.classifier.owner + if (decl.symbol == sym) { + return true + } + if (checkTypeArgs(sym, type.arguments)) { + return true + } + } + return false + } + + fun possiblyCyclicExtraction(classToCheck: IrClass, args: List): Boolean { + for (c in stack) { + if (c.symbol == classToCheck.symbol && checkTypeArgs(c.symbol, args)) { + return true + } + } + return false + } +} + diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceA.java b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceA.java new file mode 100644 index 000000000000..b8ffc44dadae --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceA.java @@ -0,0 +1,4 @@ + +package somepkg; + +public interface IfaceA extends IfaceB {} diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceB.java b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceB.java new file mode 100644 index 000000000000..6c45ba3dcb35 --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceB.java @@ -0,0 +1,4 @@ + +package somepkg; + +public interface IfaceB extends IfaceC>> {} diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceC.java b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceC.java new file mode 100644 index 000000000000..e75348f1c262 --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceC.java @@ -0,0 +1,4 @@ + +package somepkg; + +public interface IfaceC {} diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceZ.java b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceZ.java new file mode 100644 index 000000000000..b46c132b0e03 --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/somepkg/IfaceZ.java @@ -0,0 +1,6 @@ + +package somepkg; + +public interface IfaceZ { + public IfaceA someFun(); +} diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.kt b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.kt new file mode 100644 index 000000000000..55767ae75f3f --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.kt @@ -0,0 +1,5 @@ +package mypkg + +import somepkg.IfaceZ + +class SomeClass(private val myVal: IfaceZ) { } diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.py b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.py new file mode 100644 index 000000000000..daa246c8c7c8 --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.py @@ -0,0 +1,6 @@ +import commands + +def test(codeql, java_full): + codeql.database.create( + command=["kotlinc somepkg/IfaceA.java somepkg/IfaceB.java somepkg/IfaceC.java somepkg/IfaceZ.java test.kt"] + ) diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.expected b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.expected new file mode 100644 index 000000000000..cf39c2a54573 --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.expected @@ -0,0 +1,19 @@ +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA<> | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA> | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA> | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA> | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA | +| file:///!unknown-binary-location/somepkg/IfaceA.class:0:0:0:0 | IfaceA | +| file:///!unknown-binary-location/somepkg/IfaceB.class:0:0:0:0 | IfaceB | +| file:///!unknown-binary-location/somepkg/IfaceB.class:0:0:0:0 | IfaceB<> | +| file:///!unknown-binary-location/somepkg/IfaceB.class:0:0:0:0 | IfaceB | +| file:///!unknown-binary-location/somepkg/IfaceB.class:0:0:0:0 | IfaceB | +| file:///!unknown-binary-location/somepkg/IfaceB.class:0:0:0:0 | IfaceB | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC<> | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC>> | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC>> | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC>> | +| file:///!unknown-binary-location/somepkg/IfaceC.class:0:0:0:0 | IfaceC> | +| file:///!unknown-binary-location/somepkg/IfaceZ.class:0:0:0:0 | IfaceZ | diff --git a/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.ql b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.ql new file mode 100644 index 000000000000..fad3645276cb --- /dev/null +++ b/java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.ql @@ -0,0 +1,5 @@ +import java + +from Type t +where t.getName().matches("Iface%") +select t diff --git a/java/ql/test-kotlin2/library-tests/nested_types/test.kt b/java/ql/test-kotlin2/library-tests/nested_types/test.kt new file mode 100644 index 000000000000..7d5e2280b92b --- /dev/null +++ b/java/ql/test-kotlin2/library-tests/nested_types/test.kt @@ -0,0 +1,16 @@ + +import java.util.Stack; + +// Diagnostic Matches: %Making use of Stack a raw type to avoid infinite recursion% + +class MyType + +fun foo1(x: List>>>) { } + +fun foo2(x: Stack>>>) { } + +class MkT { } + +fun foo3(x: MkT>>>) { } + + diff --git a/java/ql/test-kotlin2/library-tests/nested_types/types.expected b/java/ql/test-kotlin2/library-tests/nested_types/types.expected new file mode 100644 index 000000000000..ec2e407b8b7f --- /dev/null +++ b/java/ql/test-kotlin2/library-tests/nested_types/types.expected @@ -0,0 +1,13 @@ +| file:///!unknown-binary-location/MkT.class:0:0:0:0 | MkT>>> | +| file:///!unknown-binary-location/MkT.class:0:0:0:0 | MkT>> | +| file:///!unknown-binary-location/MkT.class:0:0:0:0 | MkT> | +| file:///!unknown-binary-location/MkT.class:0:0:0:0 | MkT | +| file:///modules/java.base/java/util/List.class:0:0:0:0 | List>>> | +| file:///modules/java.base/java/util/List.class:0:0:0:0 | List>> | +| file:///modules/java.base/java/util/List.class:0:0:0:0 | List> | +| file:///modules/java.base/java/util/List.class:0:0:0:0 | List | +| file:///modules/java.base/java/util/List.class:0:0:0:0 | List> | +| file:///modules/java.base/java/util/Stack.class:0:0:0:0 | Stack | +| file:///modules/java.base/java/util/Stack.class:0:0:0:0 | Stack> | +| file:///modules/java.base/java/util/Stack.class:0:0:0:0 | Stack>> | +| file:///modules/java.base/java/util/Stack.class:0:0:0:0 | Stack>>> | diff --git a/java/ql/test-kotlin2/library-tests/nested_types/types.ql b/java/ql/test-kotlin2/library-tests/nested_types/types.ql new file mode 100644 index 000000000000..3499ae037c70 --- /dev/null +++ b/java/ql/test-kotlin2/library-tests/nested_types/types.ql @@ -0,0 +1,7 @@ +import java + +from Type t +where + t.getName().matches("%MyType%") and + t.getName().matches(["List<%", "Stack<%", "MkT<%"]) +select t