MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-19 13:02:30 +07:00
Code Issues Packages Projects Releases Wiki Activity

[uast-inspections] IDEA-335116 - Non-safe string, support qualifier cleaners

Browse Source 
GitOrigin-RevId: 5bd6b1c53c32e096a7a20e7cdd23b2e39bc19fea
This commit is contained in:
Mikhail Pyltsin
2023-10-17 14:42:53 +02:00
committed by intellij-monorepo-bot
parent eb6980f7c3
commit de1dca93d3
5 changed files with 143 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
jvm/jvm-analysis-impl/src/com/intellij/codeInspection/sourceToSink/SourceToSinkFlowInspection.kt
View File

@@ -71,7 +71,7 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
@TestOnly
fun getUntaintedMethodMatcher() = myUntaintedMethodMatcher
private val myTaintedMethodMatcher: MethodMatcher = MethodMatcher(false, "myTaintedMethodMatcher").finishDefault()
private val myTaintedMethodMatcher: MethodMatcher = MethodMatcher(false, "myTaintedMethodMatcher").finishDefault()
@JvmField
var myUntaintedFieldClasses: MutableList<String?> = mutableListOf()
@@ -109,6 +109,16 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
var depthNestedMethods: Int = 1
var checkedTypes: MutableList<String?> = mutableListOf("java.lang.String")
@JvmField
var qualifierCleanerClass: MutableList<String?> = mutableListOf()
@JvmField
var qualifierCleanerMethod: MutableList<String?> = mutableListOf()
@JvmField
var qualifierCleanerParams: MutableList<String?> = mutableListOf()
override fun getOptionsPane(): OptPane {
return OptPane.pane(
@@ -167,7 +177,7 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
OptPane.column("untaintedParameterWithPlacePlaceMethod",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.place.method.column.title"),
RegexValidator()),
).comment(
).comment(
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.parameters.comment")),
OptPane.checkbox("processOuterMethodAsQualifierAndArguments",
@@ -248,6 +258,7 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
).copy()
val factory = TaintValueFactory(configuration).also {
it.addQualifierCleaner(qualifierCleanerMethod, qualifierCleanerClass, qualifierCleanerParams)
it.addReturnFactory(TaintValueFactory.fromMethodResult(methodNames = myTaintedMethodMatcher.methodNamePatterns,
methodClass = myTaintedMethodMatcher.classNames,
@@ -270,7 +281,8 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
targetValue = TaintValue.TAINTED))
}
return UastHintedVisitorAdapter.create(holder.file.language,
SourceToSinkFlowVisitor(holder, factory, warnIfComplex, showUnknownObject, showUnsafeObject, checkedTypes),
SourceToSinkFlowVisitor(holder, factory, warnIfComplex, showUnknownObject, showUnsafeObject,
checkedTypes),
arrayOf(UCallExpression::class.java,
UReturnExpression::class.java,
UBinaryExpression::class.java,
@@ -339,9 +351,11 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
if (expression == null) return
val expressionType: PsiType? = expression.getExpressionType()
if(expressionType==null ||
checkedTypes.filterNotNull().all { !(expressionType.equalsToText(it) ||
(it != "java.lang.String" && isInheritor(expressionType, it))) }) return
if (expressionType == null ||
checkedTypes.filterNotNull().all {
!(expressionType.equalsToText(it) ||
(it != "java.lang.String" && isInheritor(expressionType, it)))
}) return
val annotationContext = AnnotationContext.fromExpression(expression)
val contextValue: TaintValue = factory.fromAnnotationContext(annotationContext)
if (contextValue !== TaintValue.UNTAINTED) return

55
jvm/jvm-analysis-impl/src/com/intellij/codeInspection/sourceToSink/TaintAnalyzer.kt
View File

@@ -428,9 +428,17 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
if (sourcePsi == null) return TaintValue.UNKNOWN
val variableFlow = getVariableFlow(sourcePsi)
return variableFlow.process(this, initValue, uVariable, analyzeContext, usedReference, skipAfterReference).taintValue
return variableFlow.process(this, initValue, uVariable, analyzeContext, usedReference, skipAfterReference, myTaintValueFactory).taintValue
}
private fun getVariableFlow(sourcePsi: PsiElement): VariableFlow =
CachedValuesManager.getManager(sourcePsi.project)
.getCachedValue(sourcePsi, CachedValueProvider {
val method = sourcePsi.toUElement() as? UMethod
val visitor = FlowVisitor()
method?.uastBody?.accept(visitor)
return@CachedValueProvider CachedValueProvider.Result.create(visitor.flow, PsiModificationTracker.MODIFICATION_COUNT)
})
private fun fromParam(expression: UExpression, target: PsiElement?, analyzeContext: AnalyzeContext): TaintValue? {
val psiParameter = (target as? PsiParameter) ?: return null
@@ -808,7 +816,7 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
}
if (nextExpression != null) {
if (nextExpression is UBlockExpression) {
nextExpression.expressions.acceptList(returnFinder)
processList(nextExpression.expressions, returnFinder)
}
else {
nextExpression.accept(returnFinder)
@@ -817,9 +825,12 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
returns.addAll(returnFinder.returns)
if (returnFinder.onlyInSameLevel && returnFinder.returns.isNotEmpty()) {
stop = true
return true
}
return false
return true
}
private fun processList(expressions: List<UExpression>, returnFinder: ReturnFinder) {
expressions.acceptList(returnFinder)
}
override fun visitBlockExpression(node: UBlockExpression): Boolean {
@@ -827,7 +838,7 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
return true
}
val returnFinder = ReturnFinder()
node.expressions.acceptList(returnFinder)
processList(node.expressions, returnFinder)
returns.addAll(returnFinder.returns)
onlyInSameLevel = false
return true
@@ -898,12 +909,14 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
override fun afterVisitCallExpression(node: UCallExpression) {
val javaPsi = node.resolveToUElementOfType<UMethod>()?.javaPsi
val receiver = node.receiver
if (javaPsi != null && JavaMethodContractUtil.isPure(javaPsi)) {
flow.addCleaning(receiver, node)
return
}
val receiver = node.receiver
if (receiver != null) {
checkUsages(listOf(receiver), node.valueArguments)
flow.addCleaning(receiver, node)
}
if (javaPsi != null && HardcodedContracts.isKnownNoParameterLeak(javaPsi)) {
return
@@ -931,15 +944,6 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
}
}
private fun getVariableFlow(sourcePsi: PsiElement): VariableFlow =
CachedValuesManager.getManager(sourcePsi.project)
.getCachedValue(sourcePsi, CachedValueProvider {
val method = sourcePsi.toUElement() as? UMethod
val visitor = FlowVisitor()
method?.uastBody?.accept(visitor)
return@CachedValueProvider CachedValueProvider.Result.create(visitor.flow, PsiModificationTracker.MODIFICATION_COUNT)
})
private fun equalFiles(analyzeContext: AnalyzeContext, element: UElement): Boolean {
val file = element.getContainingUFile()
return file != null && analyzeContext.file.sourcePsi == file.sourcePsi
@@ -1060,6 +1064,8 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
private class VariableFlow {
private interface VariableStep
private class CleaningStep(val node: UCallExpression) : VariableStep
private class DropLocalityStep(val dependsOn: List<UExpression?>?) : VariableStep
private data class UsagesStep(val usages: MutableSet<PsiElement> = hashSetOf()) : VariableStep
@@ -1088,6 +1094,15 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
}
}
fun addCleaning(uExpression: UExpression?, node: UCallExpression) {
if (uExpression == null) {
return
}
val currentVariable = (uExpression as? UResolvable)?.resolveToUElementOfType<UVariable>() ?: return
val psiElement = currentVariable.sourcePsi ?: return
variables.putValue(psiElement, CleaningStep(node))
}
fun addAssign(currentVariable: UVariable, assign: UExpression) {
val psiElement = currentVariable.sourcePsi ?: return
variables.putValue(psiElement, AssignmentStep(assign))
@@ -1109,7 +1124,8 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
uVariable: UVariable,
analyzeContext: AnalyzeContext,
usedReference: UExpression?,
skipAfterReference: Boolean): FlowResult {
skipAfterReference: Boolean,
taintValueFactory: TaintValueFactory): FlowResult {
val psiElement = uVariable.sourcePsi ?: return FlowResult(TaintValue.UNKNOWN, false)
val variableSteps = variables[psiElement]
if (variableSteps.isEmpty()) return FlowResult(initValue, false)
@@ -1155,7 +1171,7 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
processValue = resultValue
}
val flowResults = variableStep.flows.map {
it.process(taintAnalyzer, resultValue, uVariable, analyzeContext, usedReference, skipAfterReference)
it.process(taintAnalyzer, resultValue, uVariable, analyzeContext, usedReference, skipAfterReference, taintValueFactory)
}
val fastExit = flowResults.firstOrNull { it.fast }
if (fastExit != null) {
@@ -1175,6 +1191,11 @@ class TaintAnalyzer(private val myTaintValueFactory: TaintValueFactory) {
val expression = variableStep.expression
resultValue = taintAnalyzer.fromExpressionWithoutCollection(expression, analyzeContext)
}
is CleaningStep -> {
if (taintValueFactory.needToCleanQualifier(variableStep.node)) {
resultValue = TaintValue.UNTAINTED
}
}
}
}

53
jvm/jvm-analysis-impl/src/com/intellij/codeInspection/sourceToSink/TaintValueFactory.kt
View File

@@ -13,10 +13,7 @@ import com.intellij.psi.search.GlobalSearchScope
import com.intellij.psi.util.InheritanceUtil
import com.intellij.psi.util.PsiUtil
import com.siyeh.ig.psiutils.MethodMatcher
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.ULocalVariable
import org.jetbrains.uast.UVariable
import org.jetbrains.uast.toUElement
import org.jetbrains.uast.*
import kotlin.streams.asSequence
@@ -28,6 +25,7 @@ class TaintValueFactory(private val myConfiguration: UntaintedConfiguration) {
private val myUnTaintedAnnotations: MutableSet<String> = HashSet()
private val customFactories: MutableList<(CustomContext) -> TaintValue?> = ArrayList()
private val customReturnFactories: MutableList<(PsiMethod, PsiClass?) -> ReturnFactoriesResult?> = ArrayList()
private val customQualifierCleaner: MutableList<(UCallExpression)->Boolean> = ArrayList()
init {
myTaintedAnnotations.addAll(myConfiguration.taintedAnnotations.filterNotNull())
myUnTaintedAnnotations.addAll(myConfiguration.unTaintedAnnotations.filterNotNull())
@@ -39,6 +37,49 @@ class TaintValueFactory(private val myConfiguration: UntaintedConfiguration) {
add(fromField(myConfiguration))
}
fun addQualifierCleaner(methodNames: List<String?>,
methodClass: List<String?>,
methodParams: List<String?>){
val fromMethodResult = fromMethodResult(methodNames, methodClass, TaintValue.UNTAINTED)
customQualifierCleaner.add { element ->
val psiMethod = element.resolve() ?: return@add false
val factoriesResult = fromMethodResult.invoke(psiMethod, null)
if (factoriesResult == null || factoriesResult.taintValue != TaintValue.UNTAINTED) {
return@add false
}
val className = factoriesResult.className
val index = methodClass.indexOf(className)
if (index < 0 || methodParams.size <= index) {
return@add false
}
val params = methodParams[index]
if (params == null) {
return@add false
}
val expectedParams = params.split(",")
val valueArguments = element.valueArguments
if (valueArguments.size != expectedParams.size) {
return@add false
}
for (i in valueArguments.indices) {
val uExpression = valueArguments[i]
if (uExpression is ULiteralExpression) {
val value = uExpression.value.toString()
if (value == expectedParams[i]) {
continue
}
else {
return@add false
}
}
else {
return@add false
}
}
return@add true
}
}
fun addReturnFactory(customReturnFactory: (PsiMethod, PsiClass?) -> ReturnFactoriesResult?) {
customReturnFactories.add(customReturnFactory)
}
@@ -274,6 +315,10 @@ class TaintValueFactory(private val myConfiguration: UntaintedConfiguration) {
return myConfiguration
}
fun needToCleanQualifier(node: UCallExpression): Boolean {
return customQualifierCleaner.any { it.invoke(node) }
}
companion object {
private fun fromField(context: UntaintedConfiguration): (PsiElement) -> TaintValue? {

26
jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/context/CleanQualifier.java Normal file
View File

@@ -0,0 +1,26 @@
package com.example.sqlinjection;
import org.checkerframework.checker.tainting.qual.Untainted;
public class CleanQualifier {
public static void test(CleanQualifier mustBeSafe) {
mustBeSafe.setSafe(true);
sink(mustBeSafe);
}
public static void test2(CleanQualifier mustBeSafe) {
mustBeSafe.setSafe(false);
sink(<warning descr="Unknown string is used as safe parameter">mustBeSafe</warning>);
}
public static void test3(CleanQualifier mustBeSafe) {
sink(<warning descr="Unknown string is used as safe parameter">mustBeSafe</warning>);
}
private void setSafe(boolean b) {
}
public static void sink(@Untainted CleanQualifier t) {
}
}

10
jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/sourceToSink/JavaSourceToSinkFlowInspectionContextTest.kt
View File

@@ -17,10 +17,15 @@ class JavaSourceToSinkFlowInspectionContextTest : SourceToSinkFlowInspectionTest
untaintedParameterWithPlacePlaceClass.add("com.example.sqlinjection.Complete.HttpServletResponse")
untaintedParameterWithPlacePlaceMethod.add("getWriter")
checkedTypes.add("java.util.List")
checkedTypes.add("com.example.sqlinjection.CleanQualifier")
depthInside = 10
depthOutsideMethods = 1
getUntaintedMethodMatcher().classNames.add("com.example.sqlinjection.utils.Utils")
getUntaintedMethodMatcher().methodNamePatterns.add("safe")
qualifierCleanerClass.add("com.example.sqlinjection.CleanQualifier")
qualifierCleanerMethod.add("setSafe")
qualifierCleanerParams.add("true")
}
override fun getBasePath(): String {
@@ -57,4 +62,9 @@ class JavaSourceToSinkFlowInspectionContextTest : SourceToSinkFlowInspectionTest
prepareCheckFramework()
myFixture.testHighlighting("TaintDepth.java")
}
fun `test clean qualifier`() {
prepareCheckFramework()
myFixture.testHighlighting("CleanQualifier.java")
}
}