IJ-CR-106702 [java-inspections] IDEA-271540 Improving "Non-safe string is passed to safe method"

GitOrigin-RevId: 8ebdb915c879e980e90f9da32f02049e2801b143
This commit is contained in:
Mikhail Pyltsin
2023-05-18 14:35:12 +02:00
committed by intellij-monorepo-bot
parent 3951ab59f4
commit ac7f35e7fa
4 changed files with 41 additions and 56 deletions

View File

@@ -1,7 +1,7 @@
<html>
<body>
Reports cases when a non-safe string is passed to a method with a parameter marked with <code>@Untainted</code> annotations, returned from
annotated methods or assigned to annotated fields, parameters, or local variables. Kotlin `set` and `get` methods for fields are not
annotated methods or assigned to annotated fields, parameters, or local variables. Kotlin <code>set</code> and <code>get</code> methods for fields are not
supported as entry points.
<p>
A safe object is:
@@ -51,40 +51,6 @@ supported as entry points.
<p>
Here we have a warning since <code>s1</code> has an unknown state after <code>foo</code> call result assignment.
<!-- tooltip end -->
<ul>
<li>
<p>Use the <b>Untainted annotations</b> table to specify annotations, which will be used as <code>@Untainted</code> annotations during
the analysis.
The first annotation from this list will be used for propagation if it exists in the classpath.
</li>
<li>
<p>Use the <b>Tainted annotations</b> table to specify annotations, which will be used as <code>@Tainted</code> annotations during the
analysis
</li>
<li>
<p>If the <b>Consider external methods untainted if receivers and arguments are untainted</b> option is enabled,
then external methods outside the current class will be considered as safe if their receivers and arguments are safe.
In some cases, it is not applicable, but it can be useful for stateless classes.
If this option is disabled, then all external methods will be considered as unsafe.
</li>
<li>
<p>Use the <b>Safe classes</b> option to specify classes, expressions with this type will be considered safe
</li>
<li>
<p>Use the <b>Untainted methods</b> table to specify methods, which return only safe objects
</li>
<li>
<p>Use the <b>Untainted fields</b> table to specify fields, which contain only safe objects
</li>
<li>
<p>Use the <b>Report if case is too complex to check</b> option to specify if it is needed to report strings, which can not be checked
because of the complexity
</li>
<li>
<p>Use the <b>Consider parameters of private methods as safe</b> option to specify that parameters of private methods, otherwise they
will be considered as unknown.
</li>
</ul>
<p><small>New in 2021.2</small></p>
</body>
</html>

View File

@@ -93,14 +93,22 @@ jvm.inspections.source.unsafe.to.sink.flow.propagate.safe.toolwindow.unsafe.flow
propagated.from=Reason of Propagation:
propagated.to=Target to Propagate:
jvm.inspections.source.unsafe.to.sink.flow.tainted.annotations=Tainted annotations:
jvm.inspections.source.unsafe.to.sink.flow.tainted.annotations.comment=These annotations will be used as '@Tainted' annotations during the analysis
jvm.inspections.source.unsafe.to.sink.flow.untainted.annotations=Untainted annotations:
jvm.inspections.source.unsafe.to.sink.flow.untainted.annotations.comment=These annotations will be used as '@Untainted' annotations during the analysis. The first annotation from this list will be used for propagation if it exists in the classpath.
jvm.inspections.source.unsafe.to.sink.flow.untainted.methods=Untainted methods:
jvm.inspections.source.unsafe.to.sink.flow.untainted.methods.comment=These methods are considered to return only safe objects
jvm.inspections.source.unsafe.to.sink.flow.untainted.fields=Untainted fields:
jvm.inspections.source.unsafe.to.sink.flow.untainted.fields.comment=These fields are considered to contain only safe objects
jvm.inspections.source.unsafe.to.sink.flow.untainted.fields.name=Field Name
jvm.inspections.source.unsafe.to.sink.flow.safe.class=Safe classes:
jvm.inspections.source.unsafe.to.sink.flow.safe.class.comment=Expressions with these class types will be considered as safe
jvm.inspections.source.unsafe.to.sink.flow.untainted.process.as.qualifier.arguments=Consider external methods untainted if receivers and arguments are untainted
jvm.inspections.source.unsafe.to.sink.flow.untainted.process.as.qualifier.arguments.comment=If it is enabled, then external methods outside the current class will be considered as safe if their receivers and arguments are safe. In some cases, it is not applicable, but it can be useful for stateless classes. Otherwise, all external methods will be considered as unsafe
jvm.inspections.source.unsafe.to.sink.flow.check.warn.if.complex = Report if case is too complex to check
jvm.inspections.source.unsafe.to.sink.flow.check.warn.if.complex.comment = Enable, if it is needed to report strings, which can not be checked because of the complexity
jvm.inspections.source.unsafe.to.sink.flow.check.private.methods = Consider parameters of private methods as safe
jvm.inspections.source.unsafe.to.sink.flow.check.private.methods.comment = If it is enabled, then parameters of private methods are considered as safe, otherwise they will be considered as unknown
jvm.inspections.testonly.display.name=Test-only usage in production code
jvm.inspections.testonly.class.reference=Test-only class is referenced in production code

View File

@@ -80,16 +80,11 @@ class MarkAsSafeFix extends LocalQuickFixOnPsiElement {
catch (DeepTaintAnalyzerException e) {
return null;
}
List<PsiElement> elements = new ArrayList<>(ContainerUtil.map(taintAnalyzer.getNonMarkedElements(), e -> e.myNonMarked));
ContainerUtil.removeDuplicates(elements);
ContainerUtil.sort(elements, Comparator.comparing(element -> {
//methods are always last
if (element instanceof PsiMethod) {
return 1;
}
return 0;
}));
return elements;
return taintAnalyzer.getNonMarkedElements().stream()
.map(e -> (PsiElement)e.myNonMarked)
.distinct()
.sorted(Comparator.comparing(e -> e instanceof PsiMethod))
.toList();
}
@Override

View File

@@ -4,8 +4,12 @@ package com.intellij.codeInspection.sourceToSink
import com.intellij.analysis.JvmAnalysisBundle
import com.intellij.codeInsight.options.JavaClassValidator
import com.intellij.codeInspection.*
import com.intellij.codeInspection.options.OptDescribedComponent
import com.intellij.codeInspection.options.OptPane
import com.intellij.codeInspection.options.OptRegularComponent
import com.intellij.codeInspection.restriction.AnnotationContext
import com.intellij.openapi.util.NlsContexts
import com.intellij.openapi.util.NlsSafe
import com.intellij.psi.CommonClassNames
import com.intellij.psi.JavaPsiFacade
import com.intellij.psi.PsiElementVisitor
@@ -56,28 +60,31 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
OptPane.stringList("taintedAnnotations",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.tainted.annotations"),
JavaClassValidator().annotationsOnly()
),
).comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.tainted.annotations.comment")),
OptPane.stringList("untaintedAnnotations",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.annotations"),
JavaClassValidator().annotationsOnly()
),
).comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.annotations.comment")),
OptPane.checkbox("processOuterMethodAsQualifierAndArguments",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.process.as.qualifier.arguments")),
myUntaintedMethodMatcher.getTable(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.methods"))
.prefix("myUntaintedMethodMatcher"),
OptPane.stringList("skipClasses",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.safe.class")),
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.process.as.qualifier.arguments"))
.comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.process.as.qualifier.arguments.comment")),
myUntaintedMethodMatcher.getTable(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.methods")).prefix("myUntaintedMethodMatcher")
.comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.methods.comment")),
OptPane.stringList("skipClasses", JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.safe.class"))
.comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.safe.class.comment")),
OptPane.table(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.fields"),
OptPane.column("myUntaintedFieldClasses",
InspectionGadgetsBundle.message("result.of.method.call.ignored.class.column.title"),
JavaClassValidator()),
OptPane.column("myUntaintedFieldNames",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.fields.name"))
),
).comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.untainted.fields.comment")),
OptPane.checkbox("parameterOfPrivateMethodIsUntainted",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.check.private.methods")),
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.check.private.methods"))
.comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.check.private.methods.comment")),
OptPane.checkbox("warnIfComplex",
JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.check.warn.if.complex"))
.comment(JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.check.warn.if.complex.comment"))
)
}
@@ -101,7 +108,8 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
skipClasses = skipClasses,
parameterOfPrivateMethodIsUntainted = parameterOfPrivateMethodIsUntainted).copy()
return UastHintedVisitorAdapter.create(holder.file.language, SourceToSinkFlowVisitor(holder, TaintValueFactory(configuration), warnIfComplex),
return UastHintedVisitorAdapter.create(holder.file.language,
SourceToSinkFlowVisitor(holder, TaintValueFactory(configuration), warnIfComplex),
arrayOf(UCallExpression::class.java,
UReturnExpression::class.java,
UBinaryExpression::class.java,
@@ -208,4 +216,12 @@ class SourceToSinkFlowInspection : AbstractBaseUastLocalInspectionTool() {
super.writeSettings(element)
myUntaintedMethodMatcher.writeSettings(element)
}
}
}
private fun OptRegularComponent.comment(@NlsContexts.Tooltip @NlsSafe comment: String): OptRegularComponent {
if(this is OptDescribedComponent){
val component = this.description(comment)
return component ?: this
}
return this
}