diff --git a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/ap/ifds/analysis/alias/LocalAliasAnalysis.kt b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/ap/ifds/analysis/alias/LocalAliasAnalysis.kt index 50e564d4b..84f71197c 100644 --- a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/ap/ifds/analysis/alias/LocalAliasAnalysis.kt +++ b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/ap/ifds/analysis/alias/LocalAliasAnalysis.kt @@ -1,5 +1,6 @@ package org.opentaint.dataflow.ap.ifds.analysis.alias +import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap import org.opentaint.dataflow.ap.ifds.AccessPathBase import org.opentaint.dataflow.util.getOrCreate @@ -15,7 +16,11 @@ abstract class LocalAliasAnalysis { val aliasInfo: AnalysisResult? by lazy { compute() } - val convertedAliases = Long2ObjectOpenHashMap>() + val convertedAliases = Long2ObjectOpenHashMap>>() + + protected open val aliasCompressionThreshold: Int = Int.MAX_VALUE + + protected open fun compressAliases(aliases: List): List = aliases fun findAlias(base: AccessPathBase.LocalVar, statement: CommonInst): List? = withStateBeforeStatement(statement) { state, stateId -> state.findLocalAlias(stateId, base.idx) } @@ -120,7 +125,7 @@ abstract class LocalAliasAnalysis { result += convert(stateId, aliasIdx, depth = 0) } } - return result + return compressIfRequired(result) } private fun State.convertAllAliasSets(stateId: Int): List> = @@ -129,15 +134,19 @@ abstract class LocalAliasAnalysis { aliasSet.forEach { result += convert(stateId, it, depth = 0) } - result + compressIfRequired(result) } abstract fun convert(info: AAInfo, depth: Int, convertInstance: (Int) -> List): List private fun State.convert(stateId: Int, infoIdx: Int, depth: Int): List = synchronized(convertedAliases) { - convertedAliases.getOrCreate(pair(infoIdx, stateId)) { - convert(stateId, manager.getElementUncheck(infoIdx), depth) + val cacheId = pair(infoIdx, stateId) + val resultsByDepth = convertedAliases.getOrCreate(cacheId) { + Int2ObjectOpenHashMap() + } + resultsByDepth.getOrCreate(depth) { + compressIfRequired(convert(stateId, manager.getElementUncheck(infoIdx), depth)) } } @@ -147,9 +156,14 @@ abstract class LocalAliasAnalysis { forEachAliasInSet(instance) { instances += convert(stateId, it, depth + 1) } - instances + compressIfRequired(instances) } + private fun compressIfRequired(aliases: List): List { + if (aliases.size <= aliasCompressionThreshold) return aliases + return compressAliases(aliases) + } + private fun pair(a: Int, b: Int): Long = (a.toLong() shl 32) or (b.toLong() and 0xFFFF_FFFFL) } diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/samples/src/main/java/sample/alias/HeapAliasSample.java b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/samples/src/main/java/sample/alias/HeapAliasSample.java index d37a7ca4a..b2c02cde5 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/samples/src/main/java/sample/alias/HeapAliasSample.java +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/samples/src/main/java/sample/alias/HeapAliasSample.java @@ -15,6 +15,12 @@ static class Node { Object data; } + static class NodeExtra { + NodeExtra next; + NodeExtra prev; + Object data; + } + static void readArgField(Box box) { Object dst = box.value; sinkOneValue(dst); @@ -90,6 +96,20 @@ static void nodeTraversalData(Node node) { sinkOneValue(data); } + static void twoFieldNodeTraversalData(NodeExtra node) { + NodeExtra cur = node; + while (cur.next != null && cur.prev != null) { + if (cur.data instanceof String) { + cur = cur.next; + } + else { + cur = cur.prev; + } + } + Object data = cur.data; + sinkOneValue(data); + } + static void fieldOverwrite(Box box, Object a, Object b) { box.value = a; box.value = b; diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRLocalAliasAnalysis.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRLocalAliasAnalysis.kt index 23cba41c6..8391a28cd 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRLocalAliasAnalysis.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRLocalAliasAnalysis.kt @@ -23,6 +23,7 @@ import org.opentaint.dataflow.jvm.ap.ifds.alias.ExternalCallModelProvider.Extern import org.opentaint.dataflow.jvm.ap.ifds.alias.FieldAlias import org.opentaint.dataflow.jvm.ap.ifds.alias.JIRIntraProcAliasAnalysis import org.opentaint.dataflow.jvm.ap.ifds.alias.JIRIntraProcAliasAnalysis.Convert.convertToAliasInfo +import org.opentaint.dataflow.jvm.ap.ifds.alias.JIRAliasPathCompressor import org.opentaint.dataflow.jvm.ap.ifds.alias.LocalAlias import org.opentaint.dataflow.jvm.ap.ifds.alias.RefValue import org.opentaint.dataflow.jvm.ap.ifds.taint.TaintRulesProvider @@ -42,6 +43,7 @@ class JIRLocalAliasAnalysis( private val localVariableReachability: JIRLocalVariableReachability, private val cancellation: Cancellation, private val languageManager: JIRLanguageManager, + private val factTypeChecker: JIRFactTypeChecker, private val params: Params, ) : LocalAliasAnalysis() { data class Params( @@ -66,7 +68,16 @@ class JIRLocalAliasAnalysis( info: AAInfo, depth: Int, convertInstance: (Int) -> List - ): List = info.convertToAliasInfo(depth, null, convertInstance) + ): List = info.convertToAliasInfo(depth, null, ::isValidAccessorTransition, convertInstance) + + private fun isValidAccessorTransition(previous: AliasAccessor?, next: AliasAccessor): Boolean = + isValidAliasAccessorTransition(previous, next, factTypeChecker::typeMayHaveSubtypeOf) + + // Even small permutation sets multiply downstream IFDS facts, so compress every non-empty set. + override val aliasCompressionThreshold: Int = 0 + + override fun compressAliases(aliases: List): List = + JIRAliasPathCompressor.compress(aliases, cancellation::checkpoint) private inner class CallModelProvider : ExternalCallModelProvider { override fun provideModel(method: JIRMethod): List { @@ -107,10 +118,16 @@ class JIRLocalAliasAnalysis( private fun PositionAccessor.toAaAccessor(): AAHeapAccessor? = when (this) { is PositionAccessor.AnyFieldAccessor -> null is PositionAccessor.ElementAccessor -> ArrayAlias - is PositionAccessor.FieldAccessor -> FieldAlias( - AliasAccessor.Field(className, fieldName, fieldType), - isImmutable = false - ) + is PositionAccessor.FieldAccessor -> { + if (fieldName == "") { + null + } else { + FieldAlias( + AliasAccessor.Field(className, fieldName, fieldType), + isImmutable = false + ) + } + } } } @@ -139,3 +156,17 @@ class JIRLocalAliasAnalysis( data class AliasAllocInfo(val allocInst: Int) : AliasInfo } + +internal fun isValidAliasAccessorTransition( + previous: AliasAccessor?, + next: AliasAccessor, + typesMayOverlap: (String, String) -> Boolean, +): Boolean { + val field = next as? AliasAccessor.Field ?: return true + val previousType = when (previous) { + is AliasAccessor.Field -> previous.fieldType + is AliasAccessor.Static -> previous.typeName + is AliasAccessor.Array, null -> return true + } + return typesMayOverlap(previousType, field.className) +} diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressor.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressor.kt new file mode 100644 index 000000000..1f093e23a --- /dev/null +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressor.kt @@ -0,0 +1,63 @@ +package org.opentaint.dataflow.jvm.ap.ifds.alias + +import org.opentaint.dataflow.graph.IntGraph +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasAccessor +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasApInfo +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasInfo + +internal object JIRAliasPathCompressor { + fun compress(aliases: List, checkpoint: () -> Unit = {}): List { + checkpoint() + val result = LinkedHashSet(aliases) + val components = result.toList().connectedAccessorComponents(checkpoint) + if (components.isEmpty()) return result.toList() + + // Dropping the whole permutation-bearing fact is intentional: a shortened path would be a new alias fact. + return result.filterNot { alias -> + checkpoint() + alias is AliasApInfo && alias.accessors.containsAccessorPermutation(components) + } + } + + private fun List.connectedAccessorComponents( + checkpoint: () -> Unit, + ): Map { + val accessorIds = hashMapOf() + val graph = IntGraph() + + fun accessorId(accessor: AliasAccessor): Int = + accessorIds.getOrPut(accessor) { accessorIds.size } + + filterIsInstance().forEach { alias -> + checkpoint() + alias.accessors.zipWithNext { outer, inner -> + graph.addEdge(accessorId(outer), accessorId(inner)) + } + } + + checkpoint() + val components = graph.nonTrivialSccs().filter { it.cardinality() >= 2 } + if (components.isEmpty()) return emptyMap() + + val componentByAccessorId = IntArray(accessorIds.size) { NO_COMPONENT } + components.forEachIndexed { componentId, component -> + checkpoint() + var accessorId = component.nextSetBit(0) + while (accessorId >= 0) { + componentByAccessorId[accessorId] = componentId + accessorId = component.nextSetBit(accessorId + 1) + } + } + + return accessorIds.mapValues { (_, accessorId) -> componentByAccessorId[accessorId] } + } + + private fun List.containsAccessorPermutation( + componentByAccessor: Map, + ): Boolean = zipWithNext().any { (outer, inner) -> + val component = componentByAccessor[outer] ?: NO_COMPONENT + component != NO_COMPONENT && component == componentByAccessor[inner] + } + + private const val NO_COMPONENT = -1 +} diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRIntraProcAliasAnalysis.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRIntraProcAliasAnalysis.kt index b89cf2dea..361a6b847 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRIntraProcAliasAnalysis.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRIntraProcAliasAnalysis.kt @@ -104,6 +104,7 @@ class JIRIntraProcAliasAnalysis( fun AAInfo.convertToAliasInfo( depth: Int, cancellation: AnalysisCancellation?, + isValidAccessorTransition: (AliasAccessor?, AliasAccessor) -> Boolean, resolveHeapInstance: (Int) -> List ): List { if (this !is HeapAlias) { @@ -118,6 +119,8 @@ class JIRIntraProcAliasAnalysis( cancellation?.checkpoint() val instances = resolveHeapInstance(instance) + .filterNot { it is AliasApInfo && it.accessors.size >= HEAP_CHAIN_LIMIT } + val accessor = when (val a = this.heapAccessor) { is ArrayAlias -> AliasAccessor.Array is FieldAlias -> a.field @@ -127,7 +130,12 @@ class JIRIntraProcAliasAnalysis( return instances.mapNotNull { when (it) { is AliasAllocInfo -> return@mapNotNull null - is AliasApInfo -> AliasApInfo(it.base, it.accessors + accessor) + is AliasApInfo -> { + if (!isValidAccessorTransition(it.accessors.lastOrNull(), accessor)) { + return@mapNotNull null + } + AliasApInfo(it.base, it.accessors + accessor) + } } } } diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRAnalysisManager.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRAnalysisManager.kt index 5b5724f72..916da89a7 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRAnalysisManager.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRAnalysisManager.kt @@ -123,7 +123,7 @@ class JIRAnalysisManager( ?: JIRLocalAliasAnalysis( entryPointStatement, graph, callResolver.callResolver, taintConfig, - localVariableReachability, cancellation, this, aliasAnalysisParams + localVariableReachability, cancellation, this, factTypeChecker, aliasAnalysisParams ) } else { null @@ -328,4 +328,4 @@ class JIRAnalysisManager( val percentValue = current.toDouble() / total return String.format("%.2f", percentValue * 100) + "%" } -} \ No newline at end of file +} diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRAliasAccessorTypeValidationTest.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRAliasAccessorTypeValidationTest.kt new file mode 100644 index 000000000..1d8ba5429 --- /dev/null +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRAliasAccessorTypeValidationTest.kt @@ -0,0 +1,53 @@ +package org.opentaint.dataflow.jvm.ap.ifds + +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasAccessor +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class JIRAliasAccessorTypeValidationTest { + @Test + fun `rejects field access when receiver types cannot overlap`() { + val previous = field("java.io.File", "path", "java.lang.String") + val next = field("org.example.Settings", "tenantId", "org.example.TenantId") + + val valid = isValidAliasAccessorTransition(previous, next) { actual, required -> + assertTrue(actual == "java.lang.String") + assertTrue(required == "org.example.Settings") + false + } + + assertFalse(valid) + } + + @Test + fun `accepts field access when receiver types may overlap`() { + val previous = field("org.example.Container", "value", "org.example.HasTenant") + val next = field("org.example.Settings", "tenantId", "org.example.TenantId") + + assertTrue(isValidAliasAccessorTransition(previous, next) { _, _ -> true }) + } + + @Test + fun `validates a field following a static base`() { + val previous = AliasAccessor.Static("org.example.Settings") + val next = field("org.example.Settings", "DEFAULT", "org.example.Settings") + + assertTrue(isValidAliasAccessorTransition(previous, next) { actual, required -> + actual == required + }) + } + + @Test + fun `keeps transitions without enough type information`() { + val field = field("org.example.Settings", "tenantId", "org.example.TenantId") + val rejectAll: (String, String) -> Boolean = { _, _ -> false } + + assertTrue(isValidAliasAccessorTransition(null, field, rejectAll)) + assertTrue(isValidAliasAccessorTransition(AliasAccessor.Array, field, rejectAll)) + assertTrue(isValidAliasAccessorTransition(field, AliasAccessor.Array, rejectAll)) + } + + private fun field(className: String, fieldName: String, fieldType: String) = + AliasAccessor.Field(className, fieldName, fieldType) +} diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/AliasSampleTest.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/AliasSampleTest.kt index e713d55a4..5f21c8f4c 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/AliasSampleTest.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/AliasSampleTest.kt @@ -452,6 +452,33 @@ class AliasSampleTest : BasicTestUtils() { } } + @Test + fun `test node traversal on two fields produces field chain ending with data`() { + val method = findMethod(HEAP_SAMPLE, "twoFieldNodeTraversalData") + val aa = aaForMethod(method) + + val sink = method.findSinkCall("sinkOneValue") + val apAliases = aa.sinkArgApAliases(sink) + + assertTrue { + apAliases.any { + it.base == Argument(0) + && it.accessors.size >= 2 + && it.accessors.last().isField(FIELD_DATA) + && it.accessors.dropLast(1).all { a -> a.isField(FIELD_NEXT) } + } + } + + assertTrue { + apAliases.any { + it.base == Argument(0) + && it.accessors.size >= 2 + && it.accessors.last().isField(FIELD_DATA) + && it.accessors.dropLast(1).all { a -> a.isField(FIELD_PREV) } + } + } + } + @Test fun `test field overwrite on argument receiver`() { val method = findMethod(HEAP_SAMPLE, "fieldOverwrite") @@ -589,7 +616,10 @@ class AliasSampleTest : BasicTestUtils() { val localReachability = JIRLocalVariableReachability(method, graph, manager) val cancellation = Cancellation().also { it.activate() } - return JIRLocalAliasAnalysis(ep, graph, callResolver, noRules, localReachability, cancellation, manager, params) + return JIRLocalAliasAnalysis( + ep, graph, callResolver, noRules, + localReachability, cancellation, manager, manager.factTypeChecker, params + ) } private fun interProcParams(depth: Int) = @@ -643,6 +673,7 @@ class AliasSampleTest : BasicTestUtils() { private const val FIELD_VALUE = "value" private const val FIELD_BOX = "box" private const val FIELD_NEXT = "next" + private const val FIELD_PREV = "prev" private const val FIELD_DATA = "data" private const val FIELD_INTERPROC = "field" } diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressorTest.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressorTest.kt new file mode 100644 index 000000000..8484a3c77 --- /dev/null +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/test/kotlin/org/opentaint/dataflow/jvm/ap/ifds/alias/JIRAliasPathCompressorTest.kt @@ -0,0 +1,63 @@ +package org.opentaint.dataflow.jvm.ap.ifds.alias + +import org.opentaint.dataflow.ap.ifds.AccessPathBase.Companion.Argument +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasAccessor +import org.opentaint.dataflow.jvm.ap.ifds.JIRLocalAliasAnalysis.AliasApInfo +import kotlin.test.Test +import kotlin.test.assertEquals + +class JIRAliasPathCompressorTest { + @Test + fun `drops permutation facts and preserves unaffected facts`() { + val shortA = alias(a, exit) + val shortB = alias(b, exit) + val aliases = listOf( + shortA, + shortB, + alias(a, b, a, exit), + alias(b, a, b, exit), + ) + + val compressed = JIRAliasPathCompressor.compress(aliases) + + assertEquals(setOf(shortA, shortB), compressed.toSet()) + } + + @Test + fun `drops permutation facts instead of shortening them`() { + val aliases = listOf( + alias(a, b, a, exit), + alias(b, a, b, exit), + ) + + assertEquals(emptyList(), JIRAliasPathCompressor.compress(aliases)) + } + + @Test + fun `does not compress acyclic accessor chains`() { + val aliases = listOf( + alias(a, b, exit), + alias(b, exit), + ) + + assertEquals(aliases, JIRAliasPathCompressor.compress(aliases)) + } + + @Test + fun `does not compress single accessor self loop`() { + val aliases = listOf(alias(a, a, a, exit)) + + assertEquals(aliases, JIRAliasPathCompressor.compress(aliases)) + } + + private fun alias(vararg accessors: AliasAccessor): AliasApInfo = + AliasApInfo(Argument(0), accessors.toList()) + + private companion object { + val a = field("a") + val b = field("b") + val exit = field("exit") + + fun field(name: String) = AliasAccessor.Field("Test", name, "java.lang.Object") + } +}