diff --git a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/MethodSideEffectHandlerWithAnyAccessorRequestHandling.kt b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/MethodSideEffectHandlerWithAnyAccessorRequestHandling.kt index 9485b9cd6..8e9554b07 100644 --- a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/MethodSideEffectHandlerWithAnyAccessorRequestHandling.kt +++ b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/MethodSideEffectHandlerWithAnyAccessorRequestHandling.kt @@ -4,9 +4,11 @@ import org.opentaint.dataflow.ap.ifds.Accessor import org.opentaint.dataflow.ap.ifds.AnalysisRunner import org.opentaint.dataflow.ap.ifds.AnyAccessor import org.opentaint.dataflow.ap.ifds.ExclusionSet +import org.opentaint.dataflow.ap.ifds.FieldAccessor import org.opentaint.dataflow.ap.ifds.MethodSummaryEdgeApplicationUtils import org.opentaint.dataflow.ap.ifds.SideEffectKind import org.opentaint.dataflow.ap.ifds.access.FinalFactAp +import org.opentaint.dataflow.ap.ifds.access.InitialFactAp import org.opentaint.dataflow.ap.ifds.analysis.MethodSequentFlowFunction import org.opentaint.dataflow.ap.ifds.analysis.MethodSideEffectSummaryHandler @@ -19,20 +21,52 @@ interface MethodSideEffectHandlerWithAnyAccessorRequestHandling : MethodSideEffe kind: SideEffectKind ): Set { if (kind is TaintMarkFieldUnfoldRequest) { - when (summaryEffect) { - is MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication.SummaryApRefinement -> { - if (!summaryEffect.delta.isEmpty) { - handleMarkAfterAnyFieldRequest(summaryEffect.delta, kind) - } - } + handleUnfoldRequest(summaryEffect, kind) + } + + return super.handleZeroToFact(currentFactAp, summaryEffect, kind) + } + + /** + * A callee asks for its abstract initial fact to be unfolded when a taint mark its sink needs may + * be hidden under the abstraction. The request has to be answered on fact-to-fact edges too, not + * only on zero-to-fact ones: when the caller is itself analyzed from an initial fact -- i.e. the + * tainted object was passed into the caller as well -- the callee's side effect summary arrives + * here. Dropping it loses every sink whose condition reads a *field* of a formal parameter more + * than one frame below the source. + * + * Answered only while the request is still un-refined, i.e. its fact is the bare abstraction and + * no accessor below the parameter has been materialized yet. Fact-to-fact edges vastly outnumber + * zero-to-fact ones, and refining on all of them does not terminate in any reasonable time. + */ + override fun handleFactToFact( + currentInitialFactAp: InitialFactAp, + currentFactAp: FinalFactAp, + summaryEffect: MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication, + kind: SideEffectKind + ): Set { + if (kind is TaintMarkFieldUnfoldRequest && kind.fact.getAllAccessors().isEmpty()) { + handleUnfoldRequest(summaryEffect, kind) + } - is MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication.SummaryExclusionRefinement -> { - // taint mark requested -> mark not in initial fact, delta is empty -> mark not in fact + return super.handleFactToFact(currentInitialFactAp, currentFactAp, summaryEffect, kind) + } + + private fun handleUnfoldRequest( + summaryEffect: MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication, + request: TaintMarkFieldUnfoldRequest + ) { + when (summaryEffect) { + is MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication.SummaryApRefinement -> { + if (!summaryEffect.delta.isEmpty) { + handleMarkAfterAnyFieldRequest(summaryEffect.delta, request) } } - } - return super.handleZeroToFact(currentFactAp, summaryEffect, kind) + is MethodSummaryEdgeApplicationUtils.SummaryEdgeApplication.SummaryExclusionRefinement -> { + // taint mark requested -> mark not in initial fact, delta is empty -> mark not in fact + } + } } private fun handleMarkAfterAnyFieldRequest( @@ -41,7 +75,7 @@ interface MethodSideEffectHandlerWithAnyAccessorRequestHandling : MethodSideEffe ) { val mark = request.mark val allAccessors = delta.getAllAccessors() - if (mark !in allAccessors) return + val deltaHasMark = mark in allAccessors val startAccessors = hashSetOf() for (accessor in delta.getStartAccessors()) { @@ -56,8 +90,19 @@ interface MethodSideEffectHandlerWithAnyAccessorRequestHandling : MethodSideEffe anySuccessors.filterTo(startAccessors) { it !is AnyAccessor } } - val relevantStartAccessors = startAccessors.filter { accessor -> - accessor == mark || delta.readAccessor(accessor)?.getAllAccessors()?.contains(mark) ?: false + // When the caller already knows where the mark sits, refine on exactly that branch. When it + // does not -- because the caller is analyzed abstractly too and only knows the *shape* the + // value takes below the callee's parameter -- refine on that shape instead, so the callee + // materializes the accessor and can answer once the mark arrives from further up. Only a + // single concrete field qualifies: that is the shape a field-sensitive library model produces + // (`file.path`, `bean.url`), and fanning out over several accessors, or over elements, + // re-analyzes far too much of the program for the chance of finding the mark. + val relevantStartAccessors = if (deltaHasMark) { + startAccessors.filter { accessor -> + accessor == mark || delta.readAccessor(accessor)?.getAllAccessors()?.contains(mark) ?: false + } + } else { + startAccessors.filter { it is FieldAccessor }.takeIf { it.size == 1 }.orEmpty() } if (relevantStartAccessors.isEmpty()) return diff --git a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/TaintUtil.kt b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/TaintUtil.kt index b9979319d..79f02d5f2 100644 --- a/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/TaintUtil.kt +++ b/core/opentaint-dataflow-core/opentaint-dataflow/src/main/kotlin/org/opentaint/dataflow/taint/TaintUtil.kt @@ -23,8 +23,6 @@ abstract class TaintUtil(val apManager: ApManager) { abstract fun handleReachedSink(rule: Sink, factReader: FinalFactReader?, evaluatedFacts: List) - open fun patchSinkConditionFactReader(factReaders: List): List = factReaders - fun applySinkRules( sinkRules: List>, factReader: FinalFactReader?, @@ -32,8 +30,7 @@ abstract class TaintUtil(val apManager: ApManager) { ) { if (sinkRules.isEmpty()) return - val normalConditionFactReaders = factReader?.let { conditionFact(it) }.orEmpty() - val conditionFactReaders = patchSinkConditionFactReader(normalConditionFactReaders) + val conditionFactReaders = factReader?.let { conditionFact(it) }.orEmpty() sinkRules.applyRuleWithAssumptions( apManager, @@ -45,7 +42,7 @@ abstract class TaintUtil(val apManager: ApManager) { return@applyRuleWithAssumptions } - factReader?.updateRefinement(normalConditionFactReaders) + factReader?.updateRefinement(conditionFactReaders) } diff --git a/core/opentaint-dataflow-core/opentaint-go-dataflow/src/main/kotlin/org/opentaint/dataflow/go/analysis/GoMethodCallTaintUtil.kt b/core/opentaint-dataflow-core/opentaint-go-dataflow/src/main/kotlin/org/opentaint/dataflow/go/analysis/GoMethodCallTaintUtil.kt index 81da32ce2..3a1b26d79 100644 --- a/core/opentaint-dataflow-core/opentaint-go-dataflow/src/main/kotlin/org/opentaint/dataflow/go/analysis/GoMethodCallTaintUtil.kt +++ b/core/opentaint-dataflow-core/opentaint-go-dataflow/src/main/kotlin/org/opentaint/dataflow/go/analysis/GoMethodCallTaintUtil.kt @@ -1,7 +1,5 @@ package org.opentaint.dataflow.go.analysis -import org.opentaint.dataflow.ap.ifds.AccessPathBase -import org.opentaint.dataflow.ap.ifds.ElementAccessor import org.opentaint.dataflow.ap.ifds.ExclusionSet import org.opentaint.dataflow.ap.ifds.FactTypeChecker import org.opentaint.dataflow.ap.ifds.TaintMarkAccessor @@ -17,10 +15,7 @@ import org.opentaint.dataflow.go.GoMethodCallFactMapper.mapMethodExitToReturnFlo import org.opentaint.dataflow.go.rules.GoAssignAction import org.opentaint.dataflow.go.rules.GoRuleCondition import org.opentaint.dataflow.go.rules.TaintRule -import org.opentaint.dataflow.taint.FactReader import org.opentaint.dataflow.taint.FinalFactReader -import org.opentaint.dataflow.taint.FinalFactReaderWithPrefix -import org.opentaint.dataflow.taint.PositionAccess import org.opentaint.dataflow.taint.TaintSourceActionEvaluator import org.opentaint.dataflow.taint.TaintUtil import org.opentaint.ir.go.inst.GoIRInst @@ -79,16 +74,6 @@ class GoMethodCallTaintUtil( return readers } - override fun patchSinkConditionFactReader(factReaders: List): List { - val elementWrappedReaders = factReaders.mapNotNull { reader -> - val base = reader.factAp.base as? AccessPathBase.Argument ?: return@mapNotNull null - val elementPosition = PositionAccess.Complex(PositionAccess.Simple(base), ElementAccessor) - if (!reader.containsPosition(elementPosition)) return@mapNotNull null - FinalFactReaderWithPrefix(reader, ElementAccessor) - } - return factReaders + elementWrappedReaders - } - override fun handleReachedSink( rule: TaintRule.Sink, factReader: FinalFactReader?, diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRFactTypeChecker.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRFactTypeChecker.kt index 342ef70ca..0bd3fadac 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRFactTypeChecker.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/JIRFactTypeChecker.kt @@ -2,7 +2,6 @@ package org.opentaint.dataflow.jvm.ap.ifds import it.unimi.dsi.fastutil.longs.LongLongImmutablePair import it.unimi.dsi.fastutil.longs.LongLongPair -import org.opentaint.dataflow.ap.ifds.AccessPathBase import org.opentaint.dataflow.ap.ifds.Accessor import org.opentaint.dataflow.ap.ifds.AnyAccessor import org.opentaint.dataflow.ap.ifds.ClassStaticAccessor @@ -32,7 +31,6 @@ import org.opentaint.ir.api.jvm.JIRRefType import org.opentaint.ir.api.jvm.JIRType import org.opentaint.ir.api.jvm.JIRTypeVariable import org.opentaint.ir.api.jvm.JIRUnboundWildcard -import org.opentaint.ir.api.jvm.cfg.JIRCallExpr import org.opentaint.ir.api.jvm.ext.ifArrayGetElementType import org.opentaint.ir.api.jvm.ext.isAssignable import org.opentaint.ir.api.jvm.ext.isSubClassOf @@ -192,17 +190,6 @@ class JIRFactTypeChecker(private val cp: JIRClasspath) : FactTypeChecker { return AccessorCompatibilityFilter(actualType) } - fun callArgumentMayBeArray(call: JIRCallExpr, arg: AccessPathBase.Argument): Boolean { - val argument = call.args.getOrNull(arg.idx) ?: return false - val argType = argument.type - return argType.mayBeArray() - } - - fun JIRType.mayBeArray(): Boolean { - if (this !is JIRRefType) return false - return typeMayBeArrayType(this) - } - private fun accessorActualType(accessPath: List): JIRType? { val accessor = accessPath.lastOrNull() ?: return null return when (accessor) { diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallFlowFunction.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallFlowFunction.kt index 5e537cfd4..0487db76e 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallFlowFunction.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallFlowFunction.kt @@ -162,7 +162,7 @@ class JIRMethodCallFlowFunction( markAfterAnyAccessorResolver = null // we don't expect such marks in pass rules ) - val cleaner = JIRTaintCleanActionEvaluator(typeResolver) + val cleaner = JIRTaintCleanActionEvaluator() val factReaderBeforeCleaner = FinalFactReader(callerFact, apManager) val cleanRules = taintCtx.cleanRulesForCallStatement(statement, callExpr, returnValue, callerFact) @@ -297,11 +297,12 @@ class JIRMethodCallFlowFunction( } } - analysisContext.analysisManager.params.defaultGetModel?.run { - /*todo: fix owasp, propagate default only if passThroughFacts.isNone */ - val defaultRules = defaultPropagationRules(method) - val defaultPass = applyPassThrough(defaultRules, conditionEvaluator, passEvaluator) - passThroughFacts = passThroughFacts.merge(defaultPass) + if (passThroughFacts.isNone) { + analysisContext.analysisManager.params.defaultGetModel?.run { + val defaultRules = defaultPropagationRules(method) + val defaultPass = applyPassThrough(defaultRules, conditionEvaluator, passEvaluator) + passThroughFacts = passThroughFacts.merge(defaultPass) + } } passThroughFacts.onSome { evaluatedPass -> diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallRuleBasedSummaryRewriter.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallRuleBasedSummaryRewriter.kt index 1b2697e67..ab9310f10 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallRuleBasedSummaryRewriter.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodCallRuleBasedSummaryRewriter.kt @@ -91,7 +91,7 @@ class JIRMethodCallRuleBasedSummaryRewriter( val actionsForBase = userRuleDefinedActions[fact.base].orEmpty() if (actionsForBase.isEmpty()) return listOf(fact to startFactReader) - val cleanEvaluator = JIRTaintCleanActionEvaluator(typeResolver) + val cleanEvaluator = JIRTaintCleanActionEvaluator() val cleanedFact = actionsForBase.entries.applyCleanerActions( initial = EvaluatedCleanAction.initial(startFactReader) ) { (mark, actions), current -> diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodGetDefault.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodGetDefault.kt index 606e19d62..05fb70221 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodGetDefault.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/analysis/JIRMethodGetDefault.kt @@ -27,12 +27,21 @@ class JIRMethodGetDefault( private fun TypeName.mayBeArray(): Boolean = isArray || this == objectTypeName - private val getDefaultActions = listOf( - CopyAllMarks(from = Exact(This), to = Exact(Result)) + private fun defaultField(cls: JIRClassOrInterface): PositionAccessor.FieldAccessor = + PositionAccessor.FieldAccessor(cls.name, "", objectTypeName.typeName) + + private fun defaultPosition(cls: JIRClassOrInterface) = + PositionWithAccess(This, defaultField(cls)) + + private fun getDefaultActions(cls: JIRClassOrInterface) = listOf( + CopyAllMarks(from = Exact(defaultPosition(cls)), to = Exact(Result)) ) - private val getDefaultArrayActions = listOf( - CopyAllMarks(from = Exact(This), to = Exact(PositionWithAccess(Result, PositionAccessor.ElementAccessor))) + private fun getDefaultArrayActions(cls: JIRClassOrInterface) = listOf( + CopyAllMarks( + from = Exact(defaultPosition(cls)), + to = Exact(PositionWithAccess(Result, PositionAccessor.ElementAccessor)) + ) ) fun defaultPropagationRules(method: JIRMethod): List> { @@ -42,9 +51,9 @@ class JIRMethodGetDefault( if (!config.enableDefaultPropagationForClass(method.enclosingClass)) return emptyList() - var actions = getDefaultActions + var actions = getDefaultActions(method.enclosingClass) if (method.returnType.mayBeArray()) { - actions = actions + getDefaultArrayActions + actions = actions + getDefaultArrayActions(method.enclosingClass) } val getDefaultRule = TaintPassThrough(method, mkTrue(), actions, info = null) diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/JIRMethodCallTaintUtil.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/JIRMethodCallTaintUtil.kt index 393db42f4..f97bac092 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/JIRMethodCallTaintUtil.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/JIRMethodCallTaintUtil.kt @@ -1,7 +1,5 @@ package org.opentaint.dataflow.jvm.ap.ifds.taint -import org.opentaint.dataflow.ap.ifds.AccessPathBase -import org.opentaint.dataflow.ap.ifds.ElementAccessor import org.opentaint.dataflow.ap.ifds.ExclusionSet import org.opentaint.dataflow.ap.ifds.access.ApManager import org.opentaint.dataflow.ap.ifds.access.FinalFactAp @@ -16,10 +14,7 @@ import org.opentaint.dataflow.jvm.ap.ifds.JIRMethodCallFactMapper import org.opentaint.dataflow.jvm.ap.ifds.TaintConfigUtils.accept import org.opentaint.dataflow.jvm.ap.ifds.analysis.JIRMethodAnalysisContext import org.opentaint.dataflow.jvm.util.callee -import org.opentaint.dataflow.taint.FactReader import org.opentaint.dataflow.taint.FinalFactReader -import org.opentaint.dataflow.taint.FinalFactReaderWithPrefix -import org.opentaint.dataflow.taint.PositionAccess import org.opentaint.dataflow.taint.TaintSourceActionEvaluator import org.opentaint.dataflow.taint.TaintUtil import org.opentaint.ir.api.jvm.cfg.JIRCallExpr @@ -183,25 +178,6 @@ class JIRMethodCallTaintUtil( JIRMethodCallFactMapper.mapMethodExitToReturnFlowFact(statement, this) .singleOrNull() - override fun patchSinkConditionFactReader(factReaders: List): List { - val arrayElementFactReaders = factReaders.arrayElementConditionReaders(callExpr) - return factReaders + arrayElementFactReaders - } - - private fun List.arrayElementConditionReaders(callExpr: JIRCallExpr): List = - mapNotNull { - val base = it.factAp.base as? AccessPathBase.Argument ?: return@mapNotNull null - - if (!analysisContext.factTypeChecker.callArgumentMayBeArray(callExpr, base)) { - return@mapNotNull null - } - - val arrayElementPosition = PositionAccess.Complex(PositionAccess.Simple(base), ElementAccessor) - if (!it.containsPosition(arrayElementPosition)) return@mapNotNull null - - FinalFactReaderWithPrefix(it, ElementAccessor) - } - private inline fun storeInfo(body: () -> Unit) { if (generateTrace) return body() diff --git a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/TaintEvaluator.kt b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/TaintEvaluator.kt index 60ee38d3a..e72f6efd6 100644 --- a/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/TaintEvaluator.kt +++ b/core/opentaint-dataflow-core/opentaint-jvm-dataflow/src/main/kotlin/org/opentaint/dataflow/jvm/ap/ifds/taint/TaintEvaluator.kt @@ -21,16 +21,13 @@ import org.opentaint.dataflow.configuration.jvm.Result import org.opentaint.dataflow.configuration.jvm.This import org.opentaint.dataflow.taint.EvaluatedCleanAction import org.opentaint.dataflow.taint.PositionAccess -import org.opentaint.dataflow.taint.PositionTypeResolver import org.opentaint.dataflow.taint.TaintCleanActionEvaluator interface ConditionEvaluator { fun eval(condition: Condition): T } -class JIRTaintCleanActionEvaluator( - private val positionTypeResolver: PositionTypeResolver, -) { +class JIRTaintCleanActionEvaluator { private val evaluator = TaintCleanActionEvaluator() fun evaluate( @@ -49,28 +46,9 @@ class JIRTaintCleanActionEvaluator( ): List { val variable = action.position.resolveAp() val mark = TaintMarkAccessor(action.mark.name) - val cleaned = evaluator.removeFinalFact(initialFact, variable, mark, rule, action, action.position.cleanReach()) - - val positionType = positionTypeResolver.resolve(variable) - if (positionType?.typeName != STRING) { - return cleaned - } - - val stringBytesPosition = action.position.append(stringBytes) - val stringBytesVar = stringBytesPosition.resolveAp() - return cleaned.flatMap { f -> - evaluator.removeFinalFact(f, stringBytesVar, mark, rule, action, stringBytesPosition.cleanReach()) - } + return evaluator.removeFinalFact(initialFact, variable, mark, rule, action, action.position.cleanReach()) } - companion object { - private const val STRING = "java.lang.String" - - // todo: fix in config? - // string bytes virtual field fully reflects the string content. - // So, if we clean string, we should clean its byte content - private val stringBytes = PositionAccessor.FieldAccessor(STRING, "", "byte[]") - } } fun ActionPosition.resolveBaseAp(): AccessPathBase = when (this) { @@ -96,10 +74,6 @@ fun ActionPosition.cleanReach(): TaintCleanReach = when (this) { is ActionPosition.AnyAccessorAfter -> TaintCleanReach.ExactAndAnyField } -private fun ActionPosition.append(accessor: PositionAccessor): ActionPosition = when (this) { - is ActionPosition.Exact -> ActionPosition.Exact(PositionWithAccess(position, accessor)) - is ActionPosition.AnyAccessorAfter -> ActionPosition.AnyAccessorAfter(PositionWithAccess(position, accessor)) -} fun Position.resolveAp(): PositionAccess = resolveAp(resolveBaseAp()) diff --git a/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/rule.yaml b/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/rule.yaml new file mode 100644 index 000000000..2a1829e3b --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/rule.yaml @@ -0,0 +1,10 @@ +rules: + - id: builtin-slice-coverage + languages: [go] + severity: WARNING + message: "taint survives a changed builtin slice passthrough and reaches Sink" + mode: taint + pattern-sources: + - pattern: "BuiltinSliceCoverage.Source(...)" + pattern-sinks: + - pattern: "BuiltinSliceCoverage.Sink($X)" diff --git a/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/sample.go b/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/sample.go new file mode 100644 index 000000000..3b7823562 --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/BuiltinSliceCoverage/sample.go @@ -0,0 +1,35 @@ +package util + +func Source() string { return "tainted" } +func Sink(s string) { _ = s } + +// builtin append base slice arg(0): folded (elem->elem entry deleted, whole arg(0)->result kept). +func Positive_append_base() { + bar := Source() + s := []string{bar} + r := append(s, "x") + Sink(r[0]) +} + +// builtin append variadic arg(1): element star kept (boxed variadic element). +func Positive_append_variadic() { + bar := Source() + base := []string{"x"} + r := append(base, bar) + Sink(r[1]) +} + +// builtin copy(dst, src): folded (elem->elem deleted, whole arg(1)->arg(0) kept). +func Positive_copy() { + bar := Source() + src := []string{bar} + dst := make([]string, 1) + copy(dst, src) + Sink(dst[0]) +} + +func Negative_append_clean() { + s := []string{"safe"} + r := append(s, "x") + Sink(r[0]) +} diff --git a/core/opentaint-go-querylang/samples-go/FmtCoverage/rule.yaml b/core/opentaint-go-querylang/samples-go/FmtCoverage/rule.yaml new file mode 100644 index 000000000..99c0b9394 --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/FmtCoverage/rule.yaml @@ -0,0 +1,10 @@ +rules: + - id: fmt-coverage + languages: [go] + severity: WARNING + message: "taint survives a changed fmt passthrough and reaches Sink" + mode: taint + pattern-sources: + - pattern: "FmtCoverage.Source(...)" + pattern-sinks: + - pattern: "FmtCoverage.Sink($X)" diff --git a/core/opentaint-go-querylang/samples-go/FmtCoverage/sample.go b/core/opentaint-go-querylang/samples-go/FmtCoverage/sample.go new file mode 100644 index 000000000..db67d0dd9 --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/FmtCoverage/sample.go @@ -0,0 +1,64 @@ +package util + +import ( + "fmt" + "strings" +) + +func Source() string { return "tainted" } +func Sink(s string) { _ = s } + +// fmt.Sprint: Phase 2 removed [arg(*),'[*]']->result collapse; whole arg(*)->result kept. +func Positive_sprint() { + Sink(fmt.Sprint("p", Source())) +} + +// fmt.Sprintf +func Positive_sprintf() { + Sink(fmt.Sprintf("%s", Source())) +} + +// fmt.Sprintln +func Positive_sprintln() { + Sink(fmt.Sprintln(Source())) +} + +// fmt.Fprint: taints the writer arg(0); read it back. +func Positive_fprint() { + var b strings.Builder + fmt.Fprint(&b, Source()) + Sink(b.String()) +} + +// fmt.Fprintf / fmt.Fprintln: variadic collapse to the writer arg(0) (kept). +func Positive_fprintf() { + var b strings.Builder + fmt.Fprintf(&b, "%s", Source()) + Sink(b.String()) +} + +func Positive_fprintln() { + var b strings.Builder + fmt.Fprintln(&b, Source()) + Sink(b.String()) +} + +// fmt.Append / fmt.Appendf / fmt.Appendln: append formatted args to a []byte (arg->result). +func Positive_append() { + b := fmt.Append(nil, Source()) + Sink(string(b)) +} + +func Positive_appendf() { + b := fmt.Appendf(nil, "%s", Source()) + Sink(string(b)) +} + +func Positive_appendln() { + b := fmt.Appendln(nil, Source()) + Sink(string(b)) +} + +func Negative_clean() { + Sink(fmt.Sprint("safe", "clean")) +} diff --git a/core/opentaint-go-querylang/samples-go/SlicesCoverage/rule.yaml b/core/opentaint-go-querylang/samples-go/SlicesCoverage/rule.yaml new file mode 100644 index 000000000..e670cd87e --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/SlicesCoverage/rule.yaml @@ -0,0 +1,10 @@ +rules: + - id: slices-coverage + languages: [go] + severity: WARNING + message: "taint survives a folded slices passthrough and reaches Sink" + mode: taint + pattern-sources: + - pattern: "SlicesCoverage.Source(...)" + pattern-sinks: + - pattern: "SlicesCoverage.Sink($X)" diff --git a/core/opentaint-go-querylang/samples-go/SlicesCoverage/sample.go b/core/opentaint-go-querylang/samples-go/SlicesCoverage/sample.go new file mode 100644 index 000000000..d3834e75a --- /dev/null +++ b/core/opentaint-go-querylang/samples-go/SlicesCoverage/sample.go @@ -0,0 +1,32 @@ +package util + +import "slices" + +func Source() string { return "tainted" } +func Sink(s string) { _ = s } + +// Coverage intent for the folded slices.* passthroughs. PARKED (@Disabled): the +// stdlib slices.* functions are generic (e.g. Clone[S ~[]E, E any]) and the config +// key {package: slices, name: Clone} does not match the generic-instantiated call +// in any path -- so these entries were already INERT before the fold (verified: +// slices.Clone element flow is not detected even with the pre-fold [*] stars, in +// both the querylang harness and production). Removing their stars is therefore +// neutral. These flows are kept as documentation and light up if generic-function +// config matching is ever added to the engine. +func Positive_slices_clone() { + s := []string{Source()} + c := slices.Clone(s) + Sink(c[0]) +} + +func Positive_slices_compact() { + s := []string{Source(), Source()} + c := slices.Compact(s) + Sink(c[0]) +} + +func Positive_slices_delete() { + s := []string{Source(), "a"} + c := slices.Delete(s, 1, 2) + Sink(c[0]) +} diff --git a/core/opentaint-go-querylang/src/test/kotlin/org/opentaint/semgrep/GoSampleBasedTest.kt b/core/opentaint-go-querylang/src/test/kotlin/org/opentaint/semgrep/GoSampleBasedTest.kt index 93a23fe6d..64e43d2c1 100644 --- a/core/opentaint-go-querylang/src/test/kotlin/org/opentaint/semgrep/GoSampleBasedTest.kt +++ b/core/opentaint-go-querylang/src/test/kotlin/org/opentaint/semgrep/GoSampleBasedTest.kt @@ -87,6 +87,16 @@ class GoSampleBasedTest: GoSampleBasedTestBase("GO_SAMPLES_DIR") { @Test fun cookieValueFieldRead() = runSample("CookieValueFieldRead") + // Phase 3 config coverage: taint must survive the changed builtin/fmt passthroughs. + @Test fun builtinSliceCoverage() = runSample("BuiltinSliceCoverage", useDefaultConfig = true) + + @Test fun fmtCoverage() = runSample("FmtCoverage", useDefaultConfig = true) + + @Disabled // slices.* are generic (Clone[S ~[]E, E any]); the config key does not match the + // generic-instantiated call, so these entries were already inert before the fold (element + // flow undetected even with the pre-fold stars). Un-disable if generic config matching lands. + @Test fun slicesCoverage() = runSample("SlicesCoverage", useDefaultConfig = true) + @Disabled // todo: support struct-literal field matching (issues.md #8) @Test fun insecureCookieLiteral() = runSample("InsecureCookieLiteral") diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBeanIsolation.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBeanIsolation.java new file mode 100644 index 000000000..e071217a5 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBeanIsolation.java @@ -0,0 +1,264 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// Behavioural coverage for taint isolation between per-property vfield slots on beans +// this branch split off a shared/whole-object slot, but that never got a Positive/Negative +// pair proving the split actually holds at runtime. Every Negative sink below reads a +// SCALAR getter (String / boxed primitive / single Object) on purpose: a taint mark on an +// object's whole-object base does not flow into an array-element read, so an array-getter +// sink can pass for the wrong reason (see NegativeDateFormatSymbolsWeekdays in +// CoverageRuleStorageFixes.java, which stayed green while the underlying leak was live). +@RuleSet("phase3/CoverageBeanIsolation.yaml") +public abstract class CoverageBeanIsolation implements RuleSample { + public String ssrc() { return "tainted"; } + public void strSink(String s) {} + public void objSink(Object o) {} + + // 1. javax.naming.ldap.SortKey: attributeID vs matchingRuleID. + static class PositiveSortKeyAttributeId extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.ldap.SortKey k = new javax.naming.ldap.SortKey(ssrc(), true, "clean"); + strSink(k.getAttributeID()); + } + } + + // FIXED: javax.naming.ldap.SortKey#(String, boolean, String)'s config entry used + // to copy BOTH arg(0) (attributeId) and arg(2) (matchingRuleId) onto the field-sensitive + // slots AND onto the whole "this" object in the same entry, and both + // SortKey#getAttributeID and SortKey#getMatchingRuleID carried their own explicit + // `from: this to: result` copy line -- so either property leaked into the other getter + // unconditionally. The whole-object arms were removed from both the ctors and the + // getters, leaving only the field-sensitive slots. + static class NegativeSortKeyMatchingRuleIdNoLeak extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.ldap.SortKey k = new javax.naming.ldap.SortKey(ssrc(), true, "clean"); + strSink(k.getMatchingRuleID()); + } + } + + // 2. javax.naming.ldap.ExtendedRequest is SKIPPED: it is an interface (getID scalar + // String vs getEncodedValue byte[]), and the only public concrete JDK implementation, + // javax.naming.ldap.StartTlsRequest, is immutable -- its no-arg constructor hardcodes + // a fixed OID for getID() and getEncodedValue() always returns null, so there is no + // way to inject taint into either property without fabricating a non-JDK impl. + + // 3. javax.naming.ldap.Rdn: type vs value, both directions. + static class PositiveRdnGetType extends CoverageBeanIsolation { + @Override public void entrypoint() { + try { + javax.naming.ldap.Rdn r = new javax.naming.ldap.Rdn(ssrc(), "cleanValue"); + strSink(r.getType()); + } catch (javax.naming.InvalidNameException e) { + } + } + } + + // FIXED: javax.naming.ldap.Rdn#(String, Object)'s config entries used to only + // copy `arg(*) -> this` (whole object, no field split at all) -- there was no + // field-sensitive write of arg(0)/arg(1) into .Rdn#type/.Rdn#value for this constructor + // overload, so the whole-object mark set by the tainted type argument leaked into + // getValue() (which does read the field-sensitive .Rdn#value slot, but AnyAccessorEnabled + // also let the whole-object mark satisfy that read). The ctor now writes arg(0)/arg(1) + // field-sensitively instead, and getType() (previously unmodelled entirely) now reads + // .Rdn#type#String. + static class NegativeRdnValueNoLeakFromType extends CoverageBeanIsolation { + @Override public void entrypoint() { + try { + javax.naming.ldap.Rdn r = new javax.naming.ldap.Rdn(ssrc(), "cleanValue"); + objSink(r.getValue()); + } catch (javax.naming.InvalidNameException e) { + } + } + } + + static class PositiveRdnGetValue extends CoverageBeanIsolation { + @Override public void entrypoint() { + try { + javax.naming.ldap.Rdn r = new javax.naming.ldap.Rdn("cleanType", ssrc()); + objSink(r.getValue()); + } catch (javax.naming.InvalidNameException e) { + } + } + } + + // Same root cause as NegativeRdnValueNoLeakFromType, mirrored: the constructor's + // whole-object mark (set here via arg(1), the value) leaks into getType() even though + // type and value are meant to be independent slots. + static class NegativeRdnTypeNoLeakFromValue extends CoverageBeanIsolation { + @Override public void entrypoint() { + try { + javax.naming.ldap.Rdn r = new javax.naming.ldap.Rdn("cleanType", ssrc()); + strSink(r.getType()); + } catch (javax.naming.InvalidNameException e) { + } + } + } + + // 4. javax.script.SimpleScriptContext: attribute vs bindings. getBindings(int) returns + // a Bindings object (not scalar), so per the task's own soundness rule we cannot use it + // as a Negative sink. Instead: setAttribute("k", ssrc(), ENGINE_SCOPE) must not leak + // into a DIFFERENT attribute name's getAttribute("other") read -- a sound scalar + // negative that pins the attribute slot is not a whole-object channel. + static class PositiveScriptContextAttribute extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.script.SimpleScriptContext ctx = new javax.script.SimpleScriptContext(); + ctx.setAttribute("k", ssrc(), javax.script.ScriptContext.ENGINE_SCOPE); + objSink(ctx.getAttribute("k")); + } + } + + // ACCEPTED LIMITATION (not a model bug -- do not "fix" by attempting a key-sensitive + // attribute slot): javax.script.ScriptContext#setAttribute(String, Object, int) writes + // into a single .ScriptContext#attribute#java.lang.Object vfield shared by every + // attribute name. Attribute keys are runtime strings the analyzer cannot statically + // distinguish, so setAttribute("k", tainted, scope) followed by getAttribute("other") + // is observed as tainted even though "k" and "other" are different attributes. This is + // the same accepted over-approximation as java.util.Map's MapValue slot, which + // conflates all keys of a map for the same reason (see the design doc's routing of + // keyed bags to a single HOLDER slot). It is SOUND (a real cross-key flow is never + // dropped) but imprecise (this is a false positive for genuinely distinct keys). + // javax.naming.ldap.ExtendedRequest has the same key-insensitivity shape but is + // skipped above for an unrelated reason (no injectable concrete impl); no other case + // in this file fails solely because of key-insensitivity. + + // 5. java.text.ChoiceFormat: pattern (toPattern, scalar) vs limits (getLimits, + // double[] -- not a scalar sink). ChoiceFormat's only other scalar-ish output is + // format(double), which computes a formatted string from the *limits* table, not from + // the pattern text -- it is not a read of a sibling property and would not be a sound + // "does setting pattern leak elsewhere" probe. There is no clean scalar non-leak target + // on this class, so it is covered Positive-only. + static class PositiveChoiceFormatPattern extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.ChoiceFormat cf = new java.text.ChoiceFormat("0#zero|1#one"); + cf.applyPattern(ssrc()); + strSink(cf.toPattern()); + } + } + + // 6. java.text.MessageFormat: pattern (toPattern, scalar) vs locale (getLocale, scalar + // object via objSink). + static class PositiveMessageFormatPattern extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.MessageFormat mf = new java.text.MessageFormat(ssrc()); + strSink(mf.toPattern()); + } + } + + // FIXED: java.text.MessageFormat#(String) (and its (String, Locale) and + // #applyPattern(String) siblings) used to carry `arg(0) -> this` (whole object) twin + // entries -- some `taintCopyOnly: true` -- beside the field-sensitive entry writing + // .MessageFormat#pattern#String -- the whole-object twins let the pattern taint leak + // into getLocale() via AnyAccessorEnabled. All whole-object arms were removed from the + // MessageFormat ctors/applyPattern, leaving only the field-sensitive #pattern#/#locale# + // writes. + static class NegativeMessageFormatLocaleNoLeak extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.MessageFormat mf = new java.text.MessageFormat(ssrc()); + objSink(mf.getLocale()); + } + } + + // FN check for the fix above: MessageFormat#format() must still carry the pattern + // taint into its output -- a tainted pattern reaching a formatted string is a real + // injection flow, and removing the whole-object copy must not also remove this. + static class PositiveMessageFormatFormatCarriesPattern extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.MessageFormat mf = new java.text.MessageFormat("clean {0}"); + mf.applyPattern(ssrc()); + strSink(mf.format(new Object[]{"x"})); + } + } + + // 7. java.text.DecimalFormat: pattern (toPattern, scalar) vs symbols + // (getDecimalFormatSymbols, scalar object via objSink). + static class PositiveDecimalFormatPattern extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.DecimalFormat df = new java.text.DecimalFormat(); + df.applyPattern(ssrc()); + strSink(df.toPattern()); + } + } + + // FIXED: java.text.DecimalFormat#applyPattern(String) (and its (String) and + // (String, DecimalFormatSymbols) siblings) used to carry `arg(0) -> this` (whole + // object) twin entries -- some `taintCopyOnly: true` -- beside the field-sensitive + // entries writing .DecimalFormat#pattern#String (and, deliberately, + // .DecimalFormat#symbols#DecimalFormatSymbols#internationalCurrencySymbol for + // locale-affecting pattern chars) -- the whole-object twins let the pattern taint leak + // into getDecimalFormatSymbols() via AnyAccessorEnabled. All whole-object arms were + // removed, leaving only the field-sensitive #pattern#/#symbols# writes. + static class NegativeDecimalFormatSymbolsNoLeak extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.DecimalFormat df = new java.text.DecimalFormat(); + df.applyPattern(ssrc()); + objSink(df.getDecimalFormatSymbols()); + } + } + + // FN check for the fix above: DecimalFormat#format() must still carry the pattern + // taint into its output -- a tainted pattern reaching a formatted string is a real + // injection flow, and removing the whole-object copy must not also remove this. + static class PositiveDecimalFormatFormatCarriesPattern extends CoverageBeanIsolation { + @Override public void entrypoint() { + java.text.DecimalFormat df = new java.text.DecimalFormat(); + df.applyPattern(ssrc()); + strSink(df.format(1L)); + } + } + + // 8. javax.naming.directory.SearchResult: name (getName, inherited scalar String) vs + // object (getObject, scalar Object via objSink). + // + // FIXED (was a Positive miss -- the property never propagated at all): the only config + // entry that used to match the exact SearchResult(String, Object, Attributes) 3-arg + // constructor was a generic `params: index:0 type: String` rule that wrote arg(0) into + // `.javax.naming.directory.SearchResult#name#java.lang.String`. But getName() is not + // overridden on SearchResult -- it resolves to the inherited NameClassPair#getName(), + // whose config reads from the differently-keyed + // `.javax.naming.NameClassPair#name#java.lang.Object` slot (see the sibling 4-/5-arg + // constructor overloads, which correctly re-key arg(0) into that exact NameClassPair- + // owned slot). The imprecise index-based matchers were replaced with exact per- + // constructor entries writing name/obj/attrs into the slots their readers actually use. + static class PositiveSearchResultGetName extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.directory.SearchResult sr = new javax.naming.directory.SearchResult( + ssrc(), new Object(), new javax.naming.directory.BasicAttributes()); + strSink(sr.getName()); + } + } + + static class NegativeSearchResultObjectNoLeak extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.directory.SearchResult sr = new javax.naming.directory.SearchResult( + ssrc(), new Object(), new javax.naming.directory.BasicAttributes()); + objSink(sr.getObject()); + } + } + + // 9. javax.naming.Binding: object (getObject, scalar Object via objSink) vs name + // (getName, inherited scalar String). + // + // FIXED (was a Positive miss): there used to be no passThrough config entry at all for + // javax.naming.Binding#(String, Object) (confirmed by grep across + // model/java/config/stdlib/*.yaml) -- only Binding#setObject(Object) was modeled. The + // constructor argument never reached the object field, so getObject() observed no + // taint even though Binding#setObject/#getObject are themselves correctly + // field-sensitive. All four real Binding constructor overloads now write name/className + // /obj field-sensitively into the NameClassPair#name / NameClassPair#className / + // Binding#object slots their readers already use. + static class PositiveBindingGetObject extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.Binding b = new javax.naming.Binding("cleanName", ssrc()); + objSink(b.getObject()); + } + } + + static class NegativeBindingNameNoLeak extends CoverageBeanIsolation { + @Override public void entrypoint() { + javax.naming.Binding b = new javax.naming.Binding("cleanName", ssrc()); + strSink(b.getName()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBuffers.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBuffers.java new file mode 100644 index 000000000..3314a6a7c --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageBuffers.java @@ -0,0 +1,58 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.nio.ByteBuffer; +import java.nio.CharBuffer; + +// Coverage for the java.nio buffer models after the collapse. +// Each Positive puts tainted data into a buffer and reads it back out; the +// byte[] overloads exercise the element->scalar carriers that must stay explicit. +@RuleSet("phase3/CoverageBuffers.yaml") +public abstract class CoverageBuffers implements RuleSample { + public byte[] bsrc() { return new byte[]{1}; } + public String ssrc() { return "tainted"; } + public void strSink(String s) {} + public void bytesSink(byte[] b) {} + + // java.nio.ByteBuffer#put(byte[]) : arg0 and arg0[*] -> this + static class PositivePutBytesReadArray extends CoverageBuffers { + @Override public void entrypoint() { + byte[] data = bsrc(); + ByteBuffer buf = ByteBuffer.allocate(16); + buf.put(data); + bytesSink(buf.array()); + } + } + + // java.nio.ByteBuffer#get(byte[]) : this -> arg0[*] (scalar -> element) + static class PositiveGetIntoArray extends CoverageBuffers { + @Override public void entrypoint() { + byte[] data = bsrc(); + ByteBuffer buf = ByteBuffer.allocate(16); + buf.put(data); + byte[] out = new byte[16]; + buf.get(out); + bytesSink(out); + } + } + + // java.nio.CharBuffer#put(String) then toString + static class PositiveCharBufferPutToString extends CoverageBuffers { + @Override public void entrypoint() { + CharBuffer buf = CharBuffer.allocate(16); + buf.put(ssrc()); + strSink(buf.toString()); + } + } + + // Negative: a clean buffer must not be reported. + static class NegativeCleanBuffer extends CoverageBuffers { + @Override public void entrypoint() { + ByteBuffer buf = ByteBuffer.allocate(16); + buf.put(new byte[]{2}); + bytesSink(buf.array()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageCollections.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageCollections.java new file mode 100644 index 000000000..5a9b81101 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageCollections.java @@ -0,0 +1,46 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.util.List; +import java.util.Set; + +// Phase 3 core coverage: immutable-factory element passthroughs. +// Each Positive flows taint from a source, through List.of / Set.of, into the +// collection element, then out through an element read to a sink. A Positive +// turning red means the factory passthrough dropped the element taint. +@RuleSet("phase3/CoverageCollections.yaml") +public abstract class CoverageCollections implements RuleSample { + public String ssrc() { return "tainted"; } + public void strSink(String s) {} + + // java.util.List#of(Object) : arg0 -> result.Element + static class PositiveListOf extends CoverageCollections { + @Override public void entrypoint() { + String t = ssrc(); + List l = List.of(t); + strSink(l.get(0)); + } + } + + // java.util.Set#of(Object) : arg0 -> result.Element + static class PositiveSetOf extends CoverageCollections { + @Override public void entrypoint() { + String t = ssrc(); + Set s = Set.of(t); + for (String v : s) { + strSink(v); + } + } + } + + // Negative: a clean local element must not be reported. + static class NegativeCleanListOf extends CoverageCollections { + @Override public void entrypoint() { + String t = "safe"; + List l = List.of(t); + strSink(l.get(0)); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingDirectory.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingDirectory.java new file mode 100644 index 000000000..3174115cf --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingDirectory.java @@ -0,0 +1,63 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// javax.naming.directory (java.naming JDK module) passthrough coverage. Each +// Positive flows taint from a source, through a SearchControls config passthrough, +// and back out to a sink. A Positive turning red means the config change dropped a +// real flow. +@RuleSet("phase3/CoverageNamingDirectory.yaml") +public abstract class CoverageNamingDirectory implements RuleSample { + public String[] asrc() { return new String[]{"tainted"}; } + public void arrSink(String[] s) {} + public String ssrc() { return "tainted"; } + public void strSink(String s) {} + + // SearchControls#setReturningAttributes(String[]) : arg0 -> this.returningAttributes, + // read back via getReturningAttributes() : this.returningAttributes -> result. + static class PositiveSearchControlsSetter extends CoverageNamingDirectory { + @Override public void entrypoint() { + String[] a = asrc(); + javax.naming.directory.SearchControls sc = new javax.naming.directory.SearchControls(); + sc.setReturningAttributes(a); + arrSink(sc.getReturningAttributes()); + } + } + + // SearchControls#(int,long,int,String[],boolean,boolean) : arg3 -> this.returningAttributes. + static class PositiveSearchControlsCtor extends CoverageNamingDirectory { + @Override public void entrypoint() { + javax.naming.directory.SearchControls sc = + new javax.naming.directory.SearchControls(0, 0L, 0, asrc(), false, false); + arrSink(sc.getReturningAttributes()); + } + } + + // Negative: a clean local array must not be reported. + static class NegativeCleanSearchControls extends CoverageNamingDirectory { + @Override public void entrypoint() { + String[] a = new String[]{"safe"}; + javax.naming.directory.SearchControls sc = new javax.naming.directory.SearchControls(); + sc.setReturningAttributes(a); + arrSink(sc.getReturningAttributes()); + } + } + + // NameClassPair: setName must reach getName and must NOT reach getClassName. + static class PositiveNamePropertyRoundTrip extends CoverageNamingDirectory { + @Override public void entrypoint() { + javax.naming.NameClassPair p = new javax.naming.NameClassPair("a", "b"); + p.setName(ssrc()); + strSink(p.getName()); + } + } + + static class NegativeNameDoesNotLeakToClassName extends CoverageNamingDirectory { + @Override public void entrypoint() { + javax.naming.NameClassPair p = new javax.naming.NameClassPair("a", "b"); + p.setName(ssrc()); + strSink(p.getClassName()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingLdap.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingLdap.java new file mode 100644 index 000000000..4e3a9e624 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageNamingLdap.java @@ -0,0 +1,76 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// javax.naming.ldap (java.naming JDK module) passthrough coverage. The Control-family +// ctors copy the tainted arg -> this (whole-object). We sink the constructed control +// object directly (ctrlSink), which observes that whole-object taint -- no read-back +// getter is needed (getEncodedValue is not modeled and its clone-based body does not +// propagate the field in this harness). +// ExtendedRequest#createExtendedResponse is UNTESTABLE (ExtendedRequest is an interface; +// its concrete impl StartTlsRequest has an inert reflective createExtendedResponse body). +@RuleSet("phase3/CoverageNamingLdap.yaml") +public abstract class CoverageNamingLdap implements RuleSample { + public String[] asrc() { return new String[]{"tainted"}; } + public byte[] bsrc() { return new byte[]{1}; } + public void ctrlSink(Object c) {} + + // SortControl#(String[], boolean) : arg0 -> this. + static class PositiveSortControl extends CoverageNamingLdap { + @Override public void entrypoint() { + String[] a = asrc(); + try { + javax.naming.ldap.SortControl c = new javax.naming.ldap.SortControl(a, true); + ctrlSink(c); + } catch (java.io.IOException e) { + } + } + } + + // SortResponseControl#(String, boolean, byte[]) : arg2 -> this. + static class PositiveSortResponseControl extends CoverageNamingLdap { + @Override public void entrypoint() { + byte[] b = bsrc(); + try { + javax.naming.ldap.SortResponseControl c = + new javax.naming.ldap.SortResponseControl("1.2.840.113556.1.4.474", false, b); + ctrlSink(c); + } catch (java.io.IOException e) { + } + } + } + + // BasicControl#(String, boolean, byte[]) : arg2 -> this. + static class PositiveBasicControl extends CoverageNamingLdap { + @Override public void entrypoint() { + byte[] b = bsrc(); + javax.naming.ldap.BasicControl c = + new javax.naming.ldap.BasicControl("1.2", false, b); + ctrlSink(c); + } + } + + // PagedResultsResponseControl#(String, boolean, byte[]) : arg2 -> this. + static class PositivePagedResultsResponseControl extends CoverageNamingLdap { + @Override public void entrypoint() { + byte[] b = bsrc(); + try { + javax.naming.ldap.PagedResultsResponseControl c = + new javax.naming.ldap.PagedResultsResponseControl("1.2.840.113556.1.4.319", false, b); + ctrlSink(c); + } catch (java.io.IOException e) { + } + } + } + + // Negative: a clean local byte[] must not be reported. + static class NegativeCleanBasicControl extends CoverageNamingLdap { + @Override public void entrypoint() { + byte[] b = new byte[]{0}; + javax.naming.ldap.BasicControl c = + new javax.naming.ldap.BasicControl("1.2", false, b); + ctrlSink(c); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageRuleStorageFixes.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageRuleStorageFixes.java new file mode 100644 index 000000000..04c9d14c9 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageRuleStorageFixes.java @@ -0,0 +1,181 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// Behavioural coverage for nine bugs fixed by removing the generic +// carrier slot from the Java taint-model config. Each Positive proves the flow the +// fix restored/kept working; each paired Negative proves the two properties that +// used to collide through the shared slot are still kept apart. +@RuleSet("phase3/CoverageRuleStorageFixes.yaml") +public abstract class CoverageRuleStorageFixes implements RuleSample { + public String ssrc() { return "tainted"; } + public byte[] bsrc() { return new byte[]{1}; } + public void strSink(String s) {} + public void bytesSink(byte[] b) {} + public void objSink(Object o) {} + + // 1. java.nio.ByteBuffer#wrap(byte[]) element carrier: before the fix, the + // element taint on the wrapped array was dropped by the whole-copy re-root. + static class PositiveByteBufferWrapArray extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + byte[] b = bsrc(); + java.nio.ByteBuffer buf = java.nio.ByteBuffer.wrap(b); + bytesSink(buf.array()); + } + } + + // 2. java.text.MessageFormat#format(String, Object[]) element carrier: the + // whole-copy re-rooted the array element onto a scalar result, losing it. + static class PositiveMessageFormatArrayElement extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + String s = ssrc(); + String out = java.text.MessageFormat.format("{0}", new Object[]{ s }); + strSink(out); + } + } + + // 3. javax.naming.NameClassPair: name/className/fullName used to share one slot. + static class PositiveNameClassPairGetName extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + javax.naming.NameClassPair p = new javax.naming.NameClassPair("a", "b"); + p.setName(ssrc()); + strSink(p.getName()); + } + } + + static class NegativeNameClassPairGetClassName extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + javax.naming.NameClassPair p = new javax.naming.NameClassPair("a", "b"); + p.setName(ssrc()); + strSink(p.getClassName()); + } + } + + // 4. javax.naming.Reference: the factory getters used to read the className slot. + static class PositiveReferenceGetFactoryClassName extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + javax.naming.Reference r = + new javax.naming.Reference("clean.Class", ssrc(), "http://example/"); + strSink(r.getFactoryClassName()); + } + } + + static class NegativeReferenceGetClassName extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + javax.naming.Reference r = + new javax.naming.Reference("clean.Class", ssrc(), "http://example/"); + strSink(r.getClassName()); + } + } + + // 5. javax.naming.ldap.BasicControl: getID used to leak the encoded value. + static class PositiveBasicControlGetEncodedValue extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + byte[] b = bsrc(); + javax.naming.ldap.BasicControl c = new javax.naming.ldap.BasicControl("1.2", false, b); + bytesSink(c.getEncodedValue()); + } + } + + // FAILS as of this writing (see .superpowers/sdd/e2e-fixes-report.md): the + // specific field-sensitive bug (a bogus encodedValue->oid String#bytes bridge) + // was fixed, but BasicControl# still copies arg(2) (encodedValue) onto + // the whole "this" object (0587c523d6, kept deliberately for ctrlSink(c)-style + // callers), and AnyAccessorEnabled lets that whole-object mark leak through + // getID() even though getID()'s own config is field-sensitive-only. Expected: + // no finding. Actual: a finding is reported. + static class NegativeBasicControlGetID extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + byte[] b = bsrc(); + javax.naming.ldap.BasicControl c = new javax.naming.ldap.BasicControl("1.2", false, b); + strSink(c.getID()); + } + } + + // 6. javax.naming.ldap.SortControl#(String, boolean): this constructor's + // model was deleted and restored; without it the object carries no taint. + static class PositiveSortControlStringCtor extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + try { + javax.naming.ldap.SortControl c = new javax.naming.ldap.SortControl(ssrc(), true); + objSink(c); + } catch (java.io.IOException e) { + } + } + } + + // 7. javax.script.ScriptContext#setAttribute: the model stored arg(0) (the + // attribute name) instead of arg(1) (its value), so the value never propagated. + static class PositiveScriptContextAttributeValue extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + javax.script.SimpleScriptContext ctx = new javax.script.SimpleScriptContext(); + ctx.setAttribute("k", ssrc(), javax.script.ScriptContext.ENGINE_SCOPE); + objSink(ctx.getAttribute("k")); + } + } + + // 8. java.text.DateFormatSymbols: a wildcard matcher used to route all six + // array setters into the single weekdays slot. + static class PositiveDateFormatSymbolsMonths extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DateFormatSymbols dfs = new java.text.DateFormatSymbols(); + dfs.setMonths(new String[]{ ssrc() }); + strSink(dfs.getMonths()[0]); + } + } + + static class NegativeDateFormatSymbolsWeekdays extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DateFormatSymbols dfs = new java.text.DateFormatSymbols(); + dfs.setMonths(new String[]{ ssrc() }); + strSink(dfs.getWeekdays()[0]); + } + } + + // Probes whether the generic {set.+}/{get.+} whole-object channel on + // DateFormatSymbols is still live. getLocalPatternChars returns a scalar + // String, so unlike the array getters it can observe a base-level mark. + static class NegativeDateFormatSymbolsLocalPatternChars extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DateFormatSymbols dfs = new java.text.DateFormatSymbols(); + dfs.setMonths(new String[]{ ssrc() }); + strSink(dfs.getLocalPatternChars()); + } + } + + // Companion positive case: proves the localPatternChars slot itself still + // carries taint end to end now that the generic whole-object channel above + // is closed. + static class PositiveDateFormatSymbolsLocalPatternChars extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DateFormatSymbols dfs = new java.text.DateFormatSymbols(); + dfs.setLocalPatternChars(ssrc()); + strSink(dfs.getLocalPatternChars()); + } + } + + // 9. java.text.DecimalFormatSymbols: four String setters were funnelled into + // one slot. + static class PositiveDecimalFormatSymbolsNaN extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DecimalFormatSymbols dfs = new java.text.DecimalFormatSymbols(); + dfs.setNaN(ssrc()); + strSink(dfs.getNaN()); + } + } + + // FAILS as of this writing (see .superpowers/sdd/e2e-fixes-report.md): the + // per-property setters are now field-sensitive (9a9141d5c), but that commit's + // own message says the generic `set.+` whole-object taintCopyOnly twin on + // DecimalFormatSymbols ("the bare whole-object taintCopyOnly twins are left + // untouched") is deliberately kept, and AnyAccessorEnabled lets it leak + // through any getter. Expected: no finding. Actual: a finding is reported. + static class NegativeDecimalFormatSymbolsCurrencySymbol extends CoverageRuleStorageFixes { + @Override public void entrypoint() { + java.text.DecimalFormatSymbols dfs = new java.text.DecimalFormatSymbols(); + dfs.setNaN(ssrc()); + strSink(dfs.getCurrencySymbol()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSecurity.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSecurity.java new file mode 100644 index 000000000..a5a6d1f2b --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSecurity.java @@ -0,0 +1,52 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.security.CodeSigner; +import java.security.CodeSource; +import java.security.cert.Certificate; + +// Phase 3 stdlib coverage: java.security.CodeSource passthrough entries touched +// by the redundant-star cleanup. Each Positive flows taint from a source array, +// through the CodeSource constructor field store, back out through the matching +// accessor, to a sink. A Positive turning red means the config dropped a flow. +@RuleSet("phase3/CoverageSecurity.yaml") +public abstract class CoverageSecurity implements RuleSample { + public Certificate[] certSrc() { return new Certificate[0]; } + public CodeSigner[] signerSrc() { return new CodeSigner[0]; } + + public void objSink(Object o) {} + + // java.security.CodeSource#(URL,Certificate[]) : arg1 -> this.certificates ; + // getCertificates() : this.certificates -> result. + static class PositiveCodeSourceCertificates extends CoverageSecurity { + @Override public void entrypoint() { + Certificate[] certs = certSrc(); + CodeSource cs = new CodeSource((java.net.URL) null, certs); + Certificate[] got = cs.getCertificates(); + objSink(got); + } + } + + // java.security.CodeSource#(URL,CodeSigner[]) : arg1 -> this.codeSigners ; + // getCodeSigners() : this.codeSigners -> result. + static class PositiveCodeSourceSigners extends CoverageSecurity { + @Override public void entrypoint() { + CodeSigner[] signers = signerSrc(); + CodeSource cs = new CodeSource((java.net.URL) null, signers); + CodeSigner[] got = cs.getCodeSigners(); + objSink(got); + } + } + + // Negative: a clean local certificate array must not be reported. + static class NegativeCleanCodeSource extends CoverageSecurity { + @Override public void entrypoint() { + Certificate[] certs = new Certificate[0]; + CodeSource cs = new CodeSource((java.net.URL) null, certs); + Certificate[] got = cs.getCertificates(); + objSink(got); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSql.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSql.java new file mode 100644 index 000000000..9b935a4a3 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageSql.java @@ -0,0 +1,42 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// javax.sql.rowset (java.sql.rowset JDK module) passthrough coverage. JoinRowSet is +// an interface, but the config passthrough is keyed on the interface, so calling +// through the interface type (obtained from RowSetProvider) matches it directly: +// addRowSet copies arg0 -> this, and getRowSets copies this -> result. +@RuleSet("phase3/CoverageSql.yaml") +public abstract class CoverageSql implements RuleSample { + public javax.sql.RowSet rsrc() { return null; } + public void objSink(Object o) {} + + // JoinRowSet#addRowSet(RowSet, String) : arg0 -> this, read back via getRowSets(). + static class PositiveJoinRowSetAddRowSet extends CoverageSql { + @Override public void entrypoint() { + javax.sql.RowSet r = rsrc(); + try { + javax.sql.rowset.JoinRowSet j = + javax.sql.rowset.RowSetProvider.newFactory().createJoinRowSet(); + j.addRowSet(r, "col"); + objSink(j.getRowSets()); + } catch (java.sql.SQLException e) { + } + } + } + + // Negative: a clean local RowSet must not be reported. + static class NegativeCleanJoinRowSet extends CoverageSql { + @Override public void entrypoint() { + javax.sql.RowSet r = null; + try { + javax.sql.rowset.JoinRowSet j = + javax.sql.rowset.RowSetProvider.newFactory().createJoinRowSet(); + j.addRowSet(r, "col"); + objSink(j.getRowSets()); + } catch (java.sql.SQLException e) { + } + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStarSourceGetter.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStarSourceGetter.java new file mode 100644 index 000000000..152616cda --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStarSourceGetter.java @@ -0,0 +1,42 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// Verifies the mechanism the conductor response-source stars rely on: a STARRED +// source marks every field of an object, and a field-sensitive EXTERNAL getter +// (modeled as this. -> result) must then propagate that mark to a sink. +// javax.naming.NameClassPair#getName reads the .name# slot (a real builtin +// field-sensitive getter). ncpSrc() returns a NameClassPair whose #name# is a +// constant (clean) -- the taint comes only from the source rule marking $P. +@RuleSet("phase3/CoverageStarSourceGetter.yaml") +public abstract class CoverageStarSourceGetter implements RuleSample { + public javax.naming.NameClassPair ncpSrc() { + return new javax.naming.NameClassPair("n", "c"); + } + + public javax.naming.NameClassPair ncpSrcPlain() { + return new javax.naming.NameClassPair("n", "c"); + } + + public void strSink(String s) {} + + // $*P marks every field of P (incl .name#); getName() reads .name#. + // If a starred source reaches a field-sensitive getter, this reports. + static class PositiveStarSourceReachesFieldGetter extends CoverageStarSourceGetter { + @Override public void entrypoint() { + javax.naming.NameClassPair p = ncpSrc(); + strSink(p.getName()); + } + } + + // Non-starred source marks only P's base value; getName() reads the .name# + // field, so a base-only mark must NOT reach it -- the control proving the + // star (not just any source) is what carries taint into the field getter. + static class NegativeBaseSourceMissesFieldGetter extends CoverageStarSourceGetter { + @Override public void entrypoint() { + javax.naming.NameClassPair p = ncpSrcPlain(); + strSink(p.getName()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStreams.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStreams.java new file mode 100644 index 000000000..c8a2350d5 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStreams.java @@ -0,0 +1,117 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.IntBuffer; +import java.nio.LongBuffer; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +// Phase 3 stdlib coverage: java.io / java.nio / java.util.stream passthrough +// entries touched by the redundant-star cleanup. Each Positive flows taint from +// a source, through the changed passthrough, into a holder, then back out to a +// sink. A Positive turning red means the config change dropped a real flow. +@RuleSet("phase3/CoverageStreams.yaml") +public abstract class CoverageStreams implements RuleSample { + public byte[] bsrc() { return new byte[]{1}; } + public char[] csrc() { return new char[]{'x'}; } + public int[] isrc() { return new int[]{1}; } + public long[] lsrc() { return new long[]{1L}; } + public String ssrc() { return "tainted"; } + + public void bSink(byte[] b) {} + public void cSink(char[] c) {} + public void iSink(int[] i) {} + public void lSink(long[] l) {} + public void strSink(String s) {} + + // java.io.OutputStream#write(byte[]) : arg0 -> this ; toByteArray this->result. + // Also exercises the java-io `write.*` pattern entry (same arg0->this shape). + static class PositiveOutputStreamWrite extends CoverageStreams { + @Override public void entrypoint() { + try { + byte[] b = bsrc(); + ByteArrayOutputStream o = new ByteArrayOutputStream(); + o.write(b); + bSink(o.toByteArray()); + } catch (IOException e) { + } + } + } + + // java.io.ByteArrayOutputStream#write(byte[],int,int) : arg0 -> this. + static class PositiveByteArrayOutputStreamWrite3 extends CoverageStreams { + @Override public void entrypoint() { + byte[] b = bsrc(); + ByteArrayOutputStream o = new ByteArrayOutputStream(); + o.write(b, 0, b.length); + bSink(o.toByteArray()); + } + } + + // java.nio.ByteBuffer#put(int,byte[]) : arg1 -> this.data ; array() this.data->result. + static class PositiveByteBufferPut extends CoverageStreams { + @Override public void entrypoint() { + byte[] b = bsrc(); + ByteBuffer buf = ByteBuffer.allocate(64); + buf.put(0, b); + bSink(buf.array()); + } + } + + // java.nio.CharBuffer#put(int,char[]) : arg1 -> this.data ; array() -> result. + static class PositiveCharBufferPut extends CoverageStreams { + @Override public void entrypoint() { + char[] c = csrc(); + CharBuffer cb = CharBuffer.allocate(64); + cb.put(0, c); + cSink(cb.array()); + } + } + + // java.nio.IntBuffer#put(int[]) : arg0 -> this.data ; array() -> result. + static class PositiveIntBufferPut extends CoverageStreams { + @Override public void entrypoint() { + int[] i = isrc(); + IntBuffer ib = IntBuffer.allocate(64); + ib.put(i); + iSink(ib.array()); + } + } + + // java.nio.LongBuffer#put(long[]) : arg0 -> this ; array() this->result. + static class PositiveLongBufferPut extends CoverageStreams { + @Override public void entrypoint() { + long[] l = lsrc(); + LongBuffer lb = LongBuffer.allocate(64); + lb.put(l); + lSink(lb.array()); + } + } + + // java.util.stream.Stream#of(Object) : arg0 -> result.Element. + static class PositiveStreamOf extends CoverageStreams { + @Override public void entrypoint() { + String s = ssrc(); + Stream st = Stream.of(s); + List l = st.collect(Collectors.toList()); + strSink(l.get(0)); + } + } + + // Negative: a clean local buffer must not be reported. + static class NegativeCleanByteBuffer extends CoverageStreams { + @Override public void entrypoint() { + byte[] b = new byte[]{2}; + ByteBuffer buf = ByteBuffer.allocate(64); + buf.put(0, b); + bSink(buf.array()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStringBuilders.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStringBuilders.java new file mode 100644 index 000000000..5a132e914 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStringBuilders.java @@ -0,0 +1,56 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +// Phase 3 core coverage: char[] overloads of the string-builder append/insert +// entries. Each Positive flows a tainted char[] through the builder (arg -> this) +// and reads it back via toString. StringBuilder.append(char[]) is already covered +// in StdlibCoverage; here we exercise the remaining char[] overloads. The abstract +// java.lang.AbstractStringBuilder#append/#insert entries are non-instantiable and +// are therefore covered transitively through StringBuilder / StringBuffer below. +@RuleSet("phase3/CoverageStringBuilders.yaml") +public abstract class CoverageStringBuilders implements RuleSample { + public char[] csrc() { return new char[]{'x'}; } + public void strSink(String s) {} + + // java.lang.StringBuffer#append(char[]) : arg0 -> this + static class PositiveStringBufferAppendChars extends CoverageStringBuilders { + @Override public void entrypoint() { + char[] ch = csrc(); + StringBuffer sb = new StringBuffer(); + sb.append(ch); + strSink(sb.toString()); + } + } + + // java.lang.StringBuilder#insert(int, char[]) : arg1 -> this + static class PositiveStringBuilderInsertChars extends CoverageStringBuilders { + @Override public void entrypoint() { + char[] ch = csrc(); + StringBuilder sb = new StringBuilder(); + sb.insert(0, ch); + strSink(sb.toString()); + } + } + + // java.lang.StringBuffer#insert(int, char[]) : arg1 -> this + static class PositiveStringBufferInsertChars extends CoverageStringBuilders { + @Override public void entrypoint() { + char[] ch = csrc(); + StringBuffer sb = new StringBuffer(); + sb.insert(0, ch); + strSink(sb.toString()); + } + } + + // Negative: a clean local char[] must not be reported. + static class NegativeCleanAppendChars extends CoverageStringBuilders { + @Override public void entrypoint() { + char[] ch = new char[]{'y'}; + StringBuffer sb = new StringBuffer(); + sb.append(ch); + strSink(sb.toString()); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStrings.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStrings.java new file mode 100644 index 000000000..8d12e815b --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/CoverageStrings.java @@ -0,0 +1,69 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.nio.charset.StandardCharsets; +import java.text.ChoiceFormat; +import java.text.DecimalFormat; +import java.util.Locale; + +// Phase 3 core coverage: java.lang.String factory overloads plus java.text +// pattern/setter entries. Each Positive flows taint from a source, through the +// changed passthrough, and back out to a sink. The java.text cases (ChoiceFormat +// ctor, DecimalFormat set*) rely on arg -> this whole-object taint plus a guessed +// getter accessor (AnyAccessorEnabled) to read the value back. +@RuleSet("phase3/CoverageStrings.yaml") +public abstract class CoverageStrings implements RuleSample { + public Object[] osrc() { return new Object[]{"tainted"}; } + public byte[] bsrc() { return new byte[]{1}; } + public String ssrc() { return "tainted"; } + public void strSink(String s) {} + + // java.lang.String#format(Locale, String, Object[]) : arg2 -> result + static class PositiveStringFormatLocale extends CoverageStrings { + @Override public void entrypoint() { + Object[] a = osrc(); + String s = String.format(Locale.ROOT, "%s", a); + strSink(s); + } + } + + // java.lang.String#(byte[], int, int, Charset) : arg0 -> this + static class PositiveStringInitBytesCharset extends CoverageStrings { + @Override public void entrypoint() { + byte[] b = bsrc(); + String s = new String(b, 0, b.length, StandardCharsets.UTF_8); + strSink(s); + } + } + + // java.text.ChoiceFormat#(String) : arg0 -> this (read back via toPattern) + static class PositiveChoiceFormatPattern extends CoverageStrings { + @Override public void entrypoint() { + String p = ssrc(); + ChoiceFormat cf = new ChoiceFormat(p); + strSink(cf.toPattern()); + } + } + + // java.text set.+(String) : arg0 -> this (DecimalFormat#setPositivePrefix, + // read back via getPositivePrefix) + static class PositiveDecimalFormatSetPrefix extends CoverageStrings { + @Override public void entrypoint() { + String p = ssrc(); + DecimalFormat df = new DecimalFormat(); + df.setPositivePrefix(p); + strSink(df.getPositivePrefix()); + } + } + + // Negative: a clean local value must not be reported. + static class NegativeCleanStringFormat extends CoverageStrings { + @Override public void entrypoint() { + Object[] a = new Object[]{"safe"}; + String s = String.format(Locale.ROOT, "%s", a); + strSink(s); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/java/phase3/StdlibCoverage.java b/core/opentaint-java-querylang/samples/src/main/java/phase3/StdlibCoverage.java new file mode 100644 index 000000000..b97cb2699 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/java/phase3/StdlibCoverage.java @@ -0,0 +1,55 @@ +package phase3; + +import base.RuleSample; +import base.RuleSet; + +import java.util.Arrays; + +// Phase 3 config coverage: each Positive flows taint from a source, through a +// changed passthrough entry (Phase 1 fold or Phase 2 collapse removal), to a sink. +// A Positive turning red means the config change dropped a real flow. +@RuleSet("phase3/StdlibCoverage.yaml") +public abstract class StdlibCoverage implements RuleSample { + public String[] asrc() { return new String[]{"tainted"}; } + public char[] csrc() { return new char[]{'x'}; } + public void arrSink(String[] s) {} + public void strSink(String s) {} + + // Phase 1 fold: java.util.Arrays#copyOf [arg0,*]->[result,*] => arg0->result + static class PositiveArraysCopyOf extends StdlibCoverage { + @Override public void entrypoint() { + String[] d = asrc(); + String[] c = Arrays.copyOf(d, 1); + arrSink(c); + } + } + + // Phase 1 fold: java.util.Arrays#copyOfRange + static class PositiveArraysCopyOfRange extends StdlibCoverage { + @Override public void entrypoint() { + String[] d = asrc(); + String[] c = Arrays.copyOfRange(d, 0, 1); + arrSink(c); + } + } + + // Phase 2 collapse removed: java.lang.AbstractStringBuilder#append(char[]) + // kept whole copy arg0->this; whole char[] taint must still reach the builder. + static class PositiveStringBuilderAppendChars extends StdlibCoverage { + @Override public void entrypoint() { + char[] ch = csrc(); + StringBuilder sb = new StringBuilder(); + sb.append(ch); + strSink(sb.toString()); + } + } + + // Negative: a locally-built clean array must not be reported. + static class NegativeCleanCopyOf extends StdlibCoverage { + @Override public void entrypoint() { + String[] d = new String[]{"safe"}; + String[] c = Arrays.copyOf(d, 1); + arrSink(c); + } + } +} diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBeanIsolation.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBeanIsolation.yaml new file mode 100644 index 000000000..c5a4ad01e --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBeanIsolation.yaml @@ -0,0 +1,18 @@ +rules: + - id: phase3-coverage-bean-isolation + languages: + - java + severity: ERROR + message: taint reaches sink through a bean property that should be isolated from an unrelated sibling property + mode: taint + pattern-sources: + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: objSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBuffers.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBuffers.yaml new file mode 100644 index 000000000..340f03cb7 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageBuffers.yaml @@ -0,0 +1,21 @@ +rules: + - id: phase3-coverage-buffers + languages: + - java + severity: ERROR + message: taint reaches sink through a java.nio buffer passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = bsrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: bytesSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageCollections.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageCollections.yaml new file mode 100644 index 000000000..8b16d5435 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageCollections.yaml @@ -0,0 +1,15 @@ +rules: + - id: phase3-coverage-collections + languages: + - java + severity: ERROR + message: taint reaches sink through an immutable-factory element passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingDirectory.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingDirectory.yaml new file mode 100644 index 000000000..35ccf66c1 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingDirectory.yaml @@ -0,0 +1,21 @@ +rules: + - id: phase3-coverage-naming-directory + languages: + - java + severity: ERROR + message: taint reaches sink through a javax.naming.directory passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = asrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: arrSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingLdap.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingLdap.yaml new file mode 100644 index 000000000..2294ed18a --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageNamingLdap.yaml @@ -0,0 +1,18 @@ +rules: + - id: phase3-coverage-naming-ldap + languages: + - java + severity: ERROR + message: taint reaches sink through a javax.naming.ldap passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = asrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = bsrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: ctrlSink($*Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageRuleStorageFixes.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageRuleStorageFixes.yaml new file mode 100644 index 000000000..7ede4c1b2 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageRuleStorageFixes.yaml @@ -0,0 +1,24 @@ +rules: + - id: phase3-coverage-rule-storage-fixes + languages: + - java + severity: ERROR + message: taint reaches sink through a passthrough fixed by the rule-storage cleanup + mode: taint + pattern-sources: + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = bsrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: bytesSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: objSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSecurity.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSecurity.yaml new file mode 100644 index 000000000..97d371d76 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSecurity.yaml @@ -0,0 +1,18 @@ +rules: + - id: phase3-coverage-security + languages: + - java + severity: ERROR + message: taint reaches sink through a java.security.CodeSource passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = certSrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = signerSrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: objSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSql.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSql.yaml new file mode 100644 index 000000000..35cfeb765 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageSql.yaml @@ -0,0 +1,15 @@ +rules: + - id: phase3-coverage-sql + languages: + - java + severity: ERROR + message: taint reaches sink through a javax.sql.rowset passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = rsrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: objSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStarSourceGetter.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStarSourceGetter.yaml new file mode 100644 index 000000000..ef8a63584 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStarSourceGetter.yaml @@ -0,0 +1,18 @@ +rules: + - id: phase3-star-source-getter + languages: + - java + severity: ERROR + message: starred source reaches sink through a field-sensitive external getter + mode: taint + pattern-sources: + - patterns: + - pattern: $*P = ncpSrc(); + - focus-metavariable: $P + - patterns: + - pattern: $P = ncpSrcPlain(); + - focus-metavariable: $P + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStreams.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStreams.yaml new file mode 100644 index 000000000..04efcb286 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStreams.yaml @@ -0,0 +1,39 @@ +rules: + - id: phase3-coverage-streams + languages: + - java + severity: ERROR + message: taint reaches sink through a java.io / java.nio / java.util.stream passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = bsrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = csrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = isrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = lsrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: bSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: cSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: iSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: lSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStringBuilders.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStringBuilders.yaml new file mode 100644 index 000000000..d1a11d29a --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStringBuilders.yaml @@ -0,0 +1,15 @@ +rules: + - id: phase3-coverage-string-builders + languages: + - java + severity: ERROR + message: taint reaches sink through a string-builder char[] passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = csrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStrings.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStrings.yaml new file mode 100644 index 000000000..760f10e40 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/CoverageStrings.yaml @@ -0,0 +1,21 @@ +rules: + - id: phase3-coverage-strings + languages: + - java + severity: ERROR + message: taint reaches sink through a String or java.text passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = osrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = bsrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = ssrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/samples/src/main/resources/phase3/StdlibCoverage.yaml b/core/opentaint-java-querylang/samples/src/main/resources/phase3/StdlibCoverage.yaml new file mode 100644 index 000000000..04996a6d0 --- /dev/null +++ b/core/opentaint-java-querylang/samples/src/main/resources/phase3/StdlibCoverage.yaml @@ -0,0 +1,21 @@ +rules: + - id: phase3-stdlib-coverage + languages: + - java + severity: ERROR + message: taint reaches sink through a changed stdlib passthrough + mode: taint + pattern-sources: + - patterns: + - pattern: $X = asrc(); + - focus-metavariable: $X + - patterns: + - pattern: $X = csrc(); + - focus-metavariable: $X + pattern-sinks: + - patterns: + - pattern: arrSink($Y); + - focus-metavariable: $Y + - patterns: + - pattern: strSink($Y); + - focus-metavariable: $Y diff --git a/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/TaintRuleProcessing.kt b/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/TaintRuleProcessing.kt index 98c1b956d..98b829976 100644 --- a/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/TaintRuleProcessing.kt +++ b/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/TaintRuleProcessing.kt @@ -104,10 +104,11 @@ data class ProcessedTaintPassRule( data class ProcessedTaintCleanRule( val rule: R, val bySideEffect: Boolean, - val cleans: Set + val cleans: Set, + val focusMetaVars: Set ) { fun flatMap(body: (R) -> List): List> = - body(rule).map { ProcessedTaintCleanRule(it, bySideEffect, cleans) } + body(rule).map { ProcessedTaintCleanRule(it, bySideEffect, cleans, focusMetaVars) } } data class ProcessedTaintRule( @@ -140,7 +141,7 @@ private fun ProcessedTaintPassRule ProcessedTaintCleanRule.compositionStrategy( strategy: TaintRuleStrategy -) = TaintCleanCompositionStrategy(rule, bySideEffect, cleans, strategy) +) = TaintCleanCompositionStrategy(rule, bySideEffect, cleans, focusMetaVars, strategy) private fun RuleConversionCtx.generateEdgeCtx( rule: ProcessedTaintRule, @@ -299,7 +300,6 @@ fun RuleConversionCtx.prepareTaintNonSourceRules( val cleaners = rule.sanitizers.map { clean -> // todo: sanitizer by side effect - // todo: sanitizer focus metavar val generatedPos = MetavarAtom.create("generated_clean_pos") val cleanAutomata = clean.pattern.map { @@ -313,7 +313,8 @@ fun RuleConversionCtx.prepareTaintNonSourceRules( ProcessedTaintCleanRule( cleanAutomata, clean.bySideEffect == true, - taintMarks.mapTo(hashSetOf()) { it.mark } + taintMarks.mapTo(hashSetOf()) { it.mark }, + clean.pattern.metaVarInfo.focusMetaVars.mapTo(hashSetOf()) { MetavarAtom.create(it) } ) } diff --git a/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/composition/TaintCleanCompositionStrategy.kt b/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/composition/TaintCleanCompositionStrategy.kt index 93a3c49e0..549d72d55 100644 --- a/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/composition/TaintCleanCompositionStrategy.kt +++ b/core/opentaint-java-querylang/src/main/kotlin/org/opentaint/semgrep/pattern/conversion/taint/composition/TaintCleanCompositionStrategy.kt @@ -16,6 +16,7 @@ class TaintCleanCompositionStrategy( private val rule: TaintAutomataEdges, private val bySideEffect: Boolean, private val cleans: Set, + private val focusMetaVars: Set, val strategy: TaintRuleStrategy ) : TaintRuleGenerationCtx.CompositionStrategy { override fun stateClean( @@ -26,12 +27,29 @@ class TaintCleanCompositionStrategy( ): List? { if (state !in rule.automata.finalAcceptStates) return null - val cleanerPos = cleanerPositions(pos) + val cleanerPos = cleanerPositions(varName, pos) return cleans.flatMap { c -> cleanerPos.map { strategy.createCleanAction(c, it) } } } - private fun cleanerPositions(pos: PositionBaseWithModifiers?): List { + /** + * `stateClean` is invoked once per metavariable the edge accesses, so [pos] is *some* position the + * pattern mentions -- for `$URI = ($REQ).getRequestURI()` it is `Result` on one invocation and + * `This` on another. When the rule names a focus metavariable, that metavariable is the sanitized + * value and the others are only there to constrain the match, so [pos] must be emitted for the + * focus invocation alone. Emitting it for every metavariable is what made an accessor sanitizer + * clean its own receiver, i.e. untaint `request` itself. + */ + private fun isFocusPosition(varName: MetavarAtom?): Boolean { + if (focusMetaVars.isEmpty()) return true + val basics = varName?.basics ?: return false + return basics.any { basic -> focusMetaVars.any { basic in it.basics } } + } + + private fun cleanerPositions( + varName: MetavarAtom?, + pos: PositionBaseWithModifiers? + ): List { val cleanerPos = mutableListOf(PositionBase.Result.base()) if (bySideEffect) { cleanerPos += PositionBase.AnyArgument(classifier = "tainted").base() @@ -52,7 +70,8 @@ class TaintCleanCompositionStrategy( // removes the taint on the flow entering the call; it is flow-specific, so a separate use of // the same variable outside this call stays tainted. For a star clean `pos` already carries // the AnyField modifier, so this stays coherent with the plain-value arm's base. - val emitPositions = (cleanerEmitPositions + listOfNotNull(pos)).distinct() + val focusPos = pos.takeIf { isFocusPosition(varName) } + val emitPositions = (cleanerEmitPositions + listOfNotNull(focusPos)).distinct() return emitPositions } diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/AccessorSanitizerScopeTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/AccessorSanitizerScopeTest.kt new file mode 100644 index 000000000..a96065b09 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/AccessorSanitizerScopeTest.kt @@ -0,0 +1,123 @@ +package org.opentaint.semgrep + +import org.opentaint.dataflow.configuration.jvm.serialized.PositionBase +import org.opentaint.dataflow.configuration.jvm.serialized.PositionBaseWithModifiers +import org.opentaint.dataflow.configuration.jvm.serialized.SerializedItem +import org.opentaint.dataflow.configuration.jvm.serialized.SerializedTaintConfig +import org.opentaint.semgrep.pattern.SemgrepLoadTrace +import org.opentaint.semgrep.pattern.SemgrepRuleLoader +import org.opentaint.semgrep.pattern.TaintRuleFromSemgrep +import org.opentaint.semgrep.pattern.conversion.JavaLanguageStrategy +import org.opentaint.semgrep.pattern.createTaintConfig +import kotlin.io.path.Path +import kotlin.test.Test +import kotlin.test.assertTrue + +/** + * A sanitizer's `focus-metavariable` names the value that gets sanitized. Every other metavariable in + * the pattern is there to constrain the match, so a clean action must not be emitted for it. + * + * This matters for *accessor* sanitizers -- `$SAFE = ($REQ).getSomething();` focused on `$SAFE`. + * Cleaning `$REQ` as well would untaint the receiver, and `request.getRequestURI()` says nothing about + * `request.getParameter("url")`. + */ +class AccessorSanitizerScopeTest { + private fun config(ruleText: String): SerializedTaintConfig { + val trace = SemgrepLoadTrace() + val loader = SemgrepRuleLoader(listOf(JavaLanguageStrategy())) + loader.registerRuleSet(ruleText, Path("sanitizer.yaml"), Path("."), trace) + val (rule, _) = loader.loadRules().rulesWithMeta.single() + @Suppress("UNCHECKED_CAST") + return (rule as TaintRuleFromSemgrep).createTaintConfig() + } + + private fun cleanPositions(cfg: SerializedTaintConfig): List = + cfg.cleaner.orEmpty().flatMap { it.cleans }.map { it.pos } + + private fun PositionBaseWithModifiers.isThis(): Boolean = base is PositionBase.This + + private fun PositionBaseWithModifiers.isResult(): Boolean = base is PositionBase.Result + + private fun PositionBaseWithModifiers.isArgument(): Boolean = + base is PositionBase.Argument || base is PositionBase.AnyArgument + + private fun rule(sanitizer: String) = """ + rules: + - id: san + severity: NOTE + message: x + languages: [java] + mode: taint + pattern-sources: + - pattern: ${'$'}X = src(); + pattern-sanitizers: +$sanitizer + pattern-sinks: + - patterns: + - pattern: sink(${'$'}Y); + - focus-metavariable: ${'$'}Y + """.trimIndent() + + @Test + fun `focusing an accessor result does not clean the bound receiver`() { + val cfg = config( + rule( + """ + - patterns: + - pattern: ${'$'}*URI = (javax.servlet.http.HttpServletRequest ${'$'}REQ).getRequestURI(); + - focus-metavariable: ${'$'}URI + """.trimIndent().prependIndent(" ") + ) + ) + val positions = cleanPositions(cfg) + assertTrue(positions.isNotEmpty(), "expected a cleaner to be generated") + assertTrue( + positions.none { it.isThis() }, + "the receiver is only a match constraint and must stay tainted; got $positions" + ) + assertTrue(positions.all { it.isResult() }, "expected the returned value only; got $positions") + } + + @Test + fun `an accessor sanitizer with an unbound receiver cleans only the result`() { + val cfg = config(rule(" - pattern: (javax.servlet.http.HttpServletRequest).getRequestURI()")) + val positions = cleanPositions(cfg) + assertTrue(positions.isNotEmpty(), "expected a cleaner to be generated") + assertTrue(positions.all { it.isResult() }, "expected the returned value only; got $positions") + } + + @Test + fun `pass-through sanitizer still cleans the sanitized argument`() { + // Here the focus metavar *is* the argument, and cleaning that position is required: the clean + // runs on the argument-keyed fact at call-to-start, where `Result` does not exist yet. + val cfg = config( + rule( + """ + - patterns: + - pattern: clean(${'$'}C); + - focus-metavariable: ${'$'}C + """.trimIndent().prependIndent(" ") + ) + ) + val positions = cleanPositions(cfg) + assertTrue( + positions.any { it.isArgument() }, + "the sanitized argument itself must still be cleaned; got $positions" + ) + } + + @Test + fun `without a focus metavariable every matched position is still cleaned`() { + // The narrowing is deliberately scoped to rules that declare a focus metavariable. A sanitizer + // that declares none has no way to say which value it sanitizes, so it keeps the wide + // behaviour rather than silently losing clean actions. + val cfg = config( + rule(" - pattern: ${'$'}SAFE = (javax.servlet.http.HttpServletRequest ${'$'}REQ).getRequestURI();") + ) + val positions = cleanPositions(cfg) + assertTrue( + positions.any { it.isThis() }, + "expected the focus-free form to keep cleaning every matched position; got $positions" + ) + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3BeanIsolationTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3BeanIsolationTest.kt new file mode 100644 index 000000000..5364ccd40 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3BeanIsolationTest.kt @@ -0,0 +1,24 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Behavioural taint-isolation coverage for bean classes this branch split into +// per-property vfield slots, but which never got an executable Positive/Negative pair +// proving the split holds (star-config branch). configurationRequired = true loads the +// bundled model/java/config; AnyAccessorEnabled mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3BeanIsolationTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `bean property isolation coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3ConfigCoverageTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3ConfigCoverageTest.kt new file mode 100644 index 000000000..e524f6b82 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3ConfigCoverageTest.kt @@ -0,0 +1,23 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Coverage for config passthrough entries changed in the redundant-star cleanup +// (Phase 1 folds + Phase 2 collapse removals). configurationRequired = true loads +// the bundled model/java/config; AnyAccessorEnabled mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3ConfigCoverageTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `stdlib passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3CoreCoverageTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3CoreCoverageTest.kt new file mode 100644 index 000000000..fbad0c03c --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3CoreCoverageTest.kt @@ -0,0 +1,32 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Coverage for JDK stdlib passthrough entries touched by the redundant-star +// cleanup: immutable collection factories, string-builder char[] overloads, and +// String / java.text factory + setter entries. configurationRequired = true loads +// the bundled model/java/config; AnyAccessorEnabled mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3CoreCoverageTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `collection factory coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `string builder char array coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `string and text passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3IoNioCoverageTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3IoNioCoverageTest.kt new file mode 100644 index 000000000..099bcf726 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3IoNioCoverageTest.kt @@ -0,0 +1,33 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Coverage for JDK stdlib passthrough entries touched by the redundant-star +// cleanup in java-io / java-nio / java-security / java-util-stream. Each Positive +// flows taint through a changed config entry to a sink; a Positive turning red +// means the config change dropped a real flow. configurationRequired = true loads +// the bundled model/java/config; AnyAccessorEnabled mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3IoNioCoverageTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `io nio stream passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `security passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `nio buffer coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3JavaxCoverageTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3JavaxCoverageTest.kt new file mode 100644 index 000000000..a91c729a8 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3JavaxCoverageTest.kt @@ -0,0 +1,33 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Coverage for JDK javax.* passthrough config entries (java.naming, java.sql.rowset). +// Each Positive flows taint from a source, through a config passthrough, to a sink. +// configurationRequired = true loads the bundled model/java/config; AnyAccessorEnabled +// mirrors the production unroll, letting whole-object ctor taint flow back through the +// (unmodeled) getEncodedValue JDK bodies. +@TestInstance(PER_CLASS) +class Phase3JavaxCoverageTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `javax naming directory passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `javax naming ldap passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @Test + fun `javax sql rowset passthrough coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3RuleStorageFixesTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3RuleStorageFixesTest.kt new file mode 100644 index 000000000..209fdf619 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3RuleStorageFixesTest.kt @@ -0,0 +1,24 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Behavioural coverage for the nine taint bugs fixed by removing the generic +// carrier slot from the Java taint-model config (star-config branch). +// configurationRequired = true loads the bundled model/java/config; AnyAccessorEnabled +// mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3RuleStorageFixesTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `rule-storage cleanup fixes coverage`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3StarSourceGetterTest.kt b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3StarSourceGetterTest.kt new file mode 100644 index 000000000..50b10a4b7 --- /dev/null +++ b/core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/Phase3StarSourceGetterTest.kt @@ -0,0 +1,27 @@ +package org.opentaint.semgrep + +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS +import org.opentaint.semgrep.util.SampleBasedTest +import org.opentaint.semgrep.util.TestAnalysisRunner +import kotlin.test.Test + +// Verifies whether a STARRED source ($*P) propagates through a field-sensitive +// EXTERNAL getter modeled as this. -> result. This is the mechanism the +// conductor response-source stars ($*UNTRUSTED = restTemplate.exchange(...)) +// depend on: if it holds, the missing conductor findings are a MODEL gap +// (okhttp/spring getters unmodeled), not a star-mechanism gap. +// configurationRequired = true loads model/java/config; AnyAccessorEnabled +// mirrors the production unroll. +@TestInstance(PER_CLASS) +class Phase3StarSourceGetterTest : SampleBasedTest(configurationRequired = true) { + @Test + fun `star source through field-sensitive getter`() = + runTest(unrollStrategy = TestAnalysisRunner.AnyAccessorEnabled) + + @AfterAll + fun close() { + closeRunner() + } +} diff --git a/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/common/sast/dataflow/TaintAnalyzer.kt b/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/common/sast/dataflow/TaintAnalyzer.kt index 226b5ff9a..8af42901f 100644 --- a/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/common/sast/dataflow/TaintAnalyzer.kt +++ b/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/common/sast/dataflow/TaintAnalyzer.kt @@ -70,7 +70,7 @@ abstract class TaintAnalyzer( open val unrollStrategy: AnyAccessorUnrollStrategy = object : AnyAccessorUnrollStrategy { override fun unrollAccessor(accessor: Accessor): Boolean = when (accessor) { is ElementAccessor -> true - is FieldAccessor -> accessor.fieldName != "" + is FieldAccessor -> true is ClassStaticAccessor, is AnyAccessor, is FinalAccessor, diff --git a/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/jvm/sast/dataflow/rules/MethodTaintConfigurationResolver.kt b/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/jvm/sast/dataflow/rules/MethodTaintConfigurationResolver.kt index 577f89dcc..4acd293c6 100644 --- a/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/jvm/sast/dataflow/rules/MethodTaintConfigurationResolver.kt +++ b/core/opentaint-jvm-sast-dataflow/src/main/kotlin/org/opentaint/jvm/sast/dataflow/rules/MethodTaintConfigurationResolver.kt @@ -81,7 +81,6 @@ import org.opentaint.ir.api.jvm.JIRTypedMethod import org.opentaint.ir.api.jvm.PredefinedPrimitives import org.opentaint.ir.api.jvm.TypeName import org.opentaint.ir.api.jvm.ext.allSuperHierarchySequence -import org.opentaint.ir.impl.cfg.util.isArray import org.opentaint.jvm.sast.dataflow.matchedAnnotations import java.util.concurrent.atomic.AtomicInteger @@ -190,15 +189,15 @@ class MethodTaintConfigurationResolver( ctx: AnyArgSpecializationCtx, ): TaintConfigurationItem = when (this) { is SerializedRule.EntryPoint -> { - TaintEntryPointSource(method, condition, taint.flatMap { it.resolveWithArray(ctx) }, info, serializedId) + TaintEntryPointSource(method, condition, taint.flatMap { it.resolve(ctx) }, info, serializedId) } is SerializedRule.Source -> { - TaintMethodSource(method, condition, taint.flatMap { it.resolveWithArray(ctx) }, info, serializedId) + TaintMethodSource(method, condition, taint.flatMap { it.resolve(ctx) }, info, serializedId) } is SerializedRule.MethodExitSource -> { - TaintMethodExitSource(method, condition, taint.flatMap { it.resolveWithArray(ctx) }, info, serializedId) + TaintMethodExitSource(method, condition, taint.flatMap { it.resolve(ctx) }, info, serializedId) } is SerializedRule.Sink -> { @@ -552,37 +551,6 @@ class MethodTaintConfigurationResolver( pos.resolveActionPosition(ctx, annotatedWith?.asAnnotationConstraint()) .map { AssignMark(taintMarkManager.taintMark(kind), it) } - // Source actions on an array- or Object-typed position taint the element as well as the - // position itself. The starred rules that express this explicitly land in 3-rules; until - // then the duplication has to stay here or array sources lose their element taint. - private fun SerializedTaintAssignAction.resolveWithArray(ctx: AnyArgSpecializationCtx): List = - pos.resolveActionPosition(ctx, annotatedWith?.asAnnotationConstraint()) - .flatMap { it.resolveArrayActionPosition() } - .map { AssignMark(taintMarkManager.taintMark(kind), it) } - - private fun ActionPosition.resolveArrayActionPosition(): List = when (this) { - is Exact -> position.resolveArrayPosition().map { Exact(it) } - is AnyAccessorAfter -> listOf(this) - } - - private fun Position.resolveArrayPosition(): List = when (this) { - is ClassStatic -> listOf(this) - is PositionWithAccess -> base.resolveArrayPosition().map { PositionWithAccess(it, access) } - is This -> listOf(this) - is Argument -> resolveArrayPosition(this, method.parameters.getOrNull(index)?.type) - is Result -> resolveArrayPosition(this, method.returnType) - } - - private fun resolveArrayPosition(position: Position, positionType: TypeName?): List { - if (positionType == null) return listOf(position) - - if (!positionType.isArray && positionType != objectTypeName) { - return listOf(position) - } - - return listOf(position, PositionWithAccess(position, PositionAccessor.ElementAccessor)) - } - private fun SerializedTaintPassAction.resolve(ctx: AnyArgSpecializationCtx): List = from.resolveActionPosition(ctx).flatMap { fromPos -> to.resolveActionPosition(ctx).map { toPos -> diff --git a/core/src/main/kotlin/org/opentaint/jvm/sast/project/spring/SpringRuleProvider.kt b/core/src/main/kotlin/org/opentaint/jvm/sast/project/spring/SpringRuleProvider.kt index 7f3778015..20996428f 100644 --- a/core/src/main/kotlin/org/opentaint/jvm/sast/project/spring/SpringRuleProvider.kt +++ b/core/src/main/kotlin/org/opentaint/jvm/sast/project/spring/SpringRuleProvider.kt @@ -4,15 +4,10 @@ import org.opentaint.dataflow.ap.ifds.AccessPathBase import org.opentaint.dataflow.ap.ifds.ClassStaticAccessor import org.opentaint.dataflow.ap.ifds.access.FactAp import org.opentaint.dataflow.ap.ifds.access.InitialFactAp -import org.opentaint.dataflow.configuration.CommonConditionRewriter import org.opentaint.dataflow.configuration.jvm.ActionPosition import org.opentaint.dataflow.configuration.jvm.Argument -import org.opentaint.dataflow.configuration.jvm.AssignMark import org.opentaint.dataflow.configuration.jvm.ClassStatic -import org.opentaint.dataflow.configuration.jvm.Condition -import org.opentaint.dataflow.configuration.jvm.ContainsMark import org.opentaint.dataflow.configuration.jvm.CopyAllMarks -import org.opentaint.dataflow.configuration.jvm.JirCondition import org.opentaint.dataflow.configuration.jvm.Position import org.opentaint.dataflow.configuration.jvm.PositionAccessor import org.opentaint.dataflow.configuration.jvm.PositionWithAccess @@ -29,15 +24,11 @@ import org.opentaint.dataflow.configuration.jvm.TaintPassThrough import org.opentaint.dataflow.configuration.jvm.TaintStaticFieldSource import org.opentaint.dataflow.configuration.jvm.This import org.opentaint.dataflow.configuration.mkTrue -import org.opentaint.dataflow.jvm.ap.ifds.taint.ContainsMarkOnAnyField import org.opentaint.dataflow.jvm.ap.ifds.taint.TaintRulesProvider -import org.opentaint.dataflow.jvm.ap.ifds.taint.resolveBaseAp import org.opentaint.ir.api.common.CommonMethod import org.opentaint.ir.api.common.cfg.CommonInst import org.opentaint.ir.api.jvm.JIRField import org.opentaint.ir.api.jvm.JIRMethod -import org.opentaint.ir.api.jvm.TypeName -import org.opentaint.ir.impl.cfg.util.isClass class SpringRuleProvider( private val base: TaintRulesProvider, @@ -45,40 +36,7 @@ class SpringRuleProvider( ) : TaintRulesProvider by base { override fun entryPointRulesForMethod(method: CommonMethod, statement: CommonInst, fact: FactAp?, allRelevant: Boolean): Iterable { if (method is SpringGeneratedMethod) return emptyList() - - val baseRules = base.entryPointRulesForMethod(method, statement, fact, allRelevant) - if (method !is JIRMethod || method.isStatic || method.isPrivate || !method.isSpringControllerMethod()) { - return baseRules - } - - return baseRules.map { taintObjectFields(method, it) } - } - - private fun taintObjectFields(method: JIRMethod, rule: TaintEntryPointSource): TaintEntryPointSource { - val actions = rule.actionsAfter.flatMap { taintObjectFields(method, it) } - return rule.copy(actionsAfter = actions) - } - - private fun taintObjectFields(method: JIRMethod, assign: AssignMark): List { - val base = assign.position.resolveBaseAp() - if (base !is AccessPathBase.Argument) return listOf(assign) - - val paramTypeName = method.parameters.getOrNull(base.idx)?.type - ?: return emptyList() - - if (!paramTypeName.isClass) return listOf(assign) - - // todo: better handling of suspend functions - if (paramTypeName.isKotlinContinuation()) return emptyList() - - return when (val p = assign.position) { - is ActionPosition.AnyAccessorAfter -> listOf(assign) - is ActionPosition.Exact -> { - val allFieldsAssign = AssignMark(assign.mark, ActionPosition.AnyAccessorAfter(p.position)) - - listOf(assign, allFieldsAssign) - } - } + return base.entryPointRulesForMethod(method, statement, fact, allRelevant) } override fun sourceRulesForMethod(method: CommonMethod, statement: CommonInst, fact: FactAp?, allRelevant: Boolean): Iterable { @@ -241,34 +199,23 @@ class SpringRuleProvider( initialFacts: Set?, allRelevant: Boolean ): Iterable { + if (method is SpringGeneratedMethod) return emptyList() if (method !is JIRMethod || !method.isSpringControllerMethod()) { return base.sinkRulesForMethodExit(method, statement, fact, initialFacts, allRelevant) } - val allBaseRules = base.sinkRulesForMethodExit(method, statement, fact, initialFacts = null, allRelevant) - return allBaseRules.map { unfoldSpringExitObject(it) } + // Pass initialFacts = null for controller-return sinks to bypass the Z2F gate in + // JIRMethodExitRuleProvider (which drops exit rules when initialFacts is non-empty). + // Controller-return XSS sinks must still fire on F2F edges, i.e. STORED / second-order + // flows where taint enters the GET handler as an initial fact (e.g. POST writes tainted + // data into a repository, GET returns repo.findById(...)). This reproduces the load-bearing + // null bypass of the removed unfoldSpringExitObject hack; the $*VAR stars in the rules now + // handle the any-field widening that the deleted ContainsMarkRewriter used to do. + return base.sinkRulesForMethodExit(method, statement, fact, initialFacts = null, allRelevant) } - private fun unfoldSpringExitObject(rule: TaintMethodExitSink): TaintMethodExitSink = - rule.copy(condition = unfoldObjectContainsMark(position = Result, rule.condition)) - - private fun unfoldObjectContainsMark(position: Position, condition: Condition): Condition = - condition.accept(ContainsMarkRewriter(position)) - - private class ContainsMarkRewriter(val position: Position) : CommonConditionRewriter { - override fun rewriteAtom(atom: JirCondition): JirCondition { - if (atom !is ContainsMark) return atom - - if (atom.position != position) return atom - return ContainsMarkOnAnyField(position, atom.mark) - } - } - - private fun TypeName.isKotlinContinuation(): Boolean = typeName == kotlinContinuation - companion object { private const val javaObject = "java.lang.Object" - private const val kotlinContinuation = "kotlin.coroutines.Continuation" private val iterableElement = PositionAccessor.FieldAccessor( className = "java.lang.Iterable",