Skip to content

Commit

Permalink
Migrate UnpredictableHashSaltCheck to kotlin-analysis-api
Browse files Browse the repository at this point in the history
  • Loading branch information
Godin committed Feb 2, 2025
1 parent f5b0e84 commit 5d1d95b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ import com.intellij.psi.impl.source.tree.LeafPsiElement
import com.intellij.psi.util.PsiTreeUtil
import org.jetbrains.kotlin.analysis.api.KaIdeApi
import org.jetbrains.kotlin.analysis.api.resolution.KaCall
import org.jetbrains.kotlin.analysis.api.resolution.KaCallableMemberCall
import org.jetbrains.kotlin.analysis.api.resolution.KaExplicitReceiverValue
import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall
import org.jetbrains.kotlin.analysis.api.resolution.KaImplicitReceiverValue
import org.jetbrains.kotlin.analysis.api.resolution.singleCallOrNull
import org.jetbrains.kotlin.analysis.api.resolution.singleFunctionCallOrNull
import org.jetbrains.kotlin.analysis.api.resolution.successfulCallOrNull
import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
Expand Down Expand Up @@ -689,6 +691,7 @@ fun ResolvedValueArgument.isNull(bindingContext: BindingContext) = (
fun KtExpression.isPredictedNull() =
predictRuntimeValueExpression().isNull()

@Deprecated("use kotlin-analysis-api instead")
fun KtExpression.getCalleeOrUnwrappedGetMethod(bindingContext: BindingContext) =
(this as? KtDotQualifiedExpression)?.let { dotQualifiedExpression ->
(dotQualifiedExpression.selectorExpression as? KtNameReferenceExpression)?.let { nameSelector ->
Expand All @@ -699,12 +702,24 @@ fun KtExpression.getCalleeOrUnwrappedGetMethod(bindingContext: BindingContext) =
/**
* Checks whether the expression is a call, matches the FunMatchers in [STRING_TO_BYTE_FUNS] and is called on a constant string value.
*/
@Deprecated("use kotlin-analysis-api instead", ReplaceWith("this.isBytesInitializedFromString()"))
fun KtExpression.isBytesInitializedFromString(bindingContext: BindingContext) =
getCalleeOrUnwrappedGetMethod(bindingContext)?.let { callee ->
STRING_TO_BYTE_FUNS.any { it.matches(callee) } &&
(getCall(bindingContext)?.explicitReceiver as? ExpressionReceiver)?.expression?.predictRuntimeStringValue(bindingContext) != null
} ?: false

fun KtExpression.isBytesInitializedFromString(): Boolean = withKaSession {
val call = this@isBytesInitializedFromString.resolveToCall()?.successfulCallOrNull<KaCallableMemberCall<*, *>>()
?: return@withKaSession false
STRING_TO_BYTE_FUNS.any { it.matches(call) } &&
((call.partiallyAppliedSymbol.extensionReceiver ?: call.partiallyAppliedSymbol.dispatchReceiver)
as? KaExplicitReceiverValue)
?.expression
?.predictRuntimeStringValue() != null
}

@Deprecated("use kotlin-analysis-api instead")
fun ResolvedCall<*>.simpleArgExpressionOrNull(index: Int) =
this.valueArgumentsByIndex
?.getOrNull(index)
Expand Down Expand Up @@ -762,12 +777,22 @@ fun KtProperty.findUsages(
/**
* Checks whether the variable has been initialized with the help of a secure random function
*/
@Deprecated("use kotlin-analysis-api instead", ReplaceWith("this.isInitializedPredictably(searchStartNode)"))
fun KtExpression.isInitializedPredictably(searchStartNode: KtExpression, bindingContext: BindingContext): Boolean {
return this !is KtNameReferenceExpression || this.findUsages(searchStartNode) {
it.getParentOfType<KtCallExpression>(false).getResolvedCall(bindingContext) matches SECURE_RANDOM_FUNS
}.isEmpty()
}

fun KtExpression.isInitializedPredictably(searchStartNode: KtExpression): Boolean {
return this !is KtNameReferenceExpression || this.findUsages(searchStartNode) {
withKaSession {
it.getParentOfType<KtCallExpression>(false)?.resolveToCall()
?.successfulFunctionCallOrNull() matches SECURE_RANDOM_FUNS
}
}.isEmpty()
}

/**
* Checks if an expression is a function local variable
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,24 @@
*/
package org.sonarsource.kotlin.checks

import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall
import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
import org.sonar.check.Rule
import org.sonarsource.kotlin.api.checks.BYTE_ARRAY_CONSTRUCTOR
import org.sonarsource.kotlin.api.checks.BYTE_ARRAY_CONSTRUCTOR_SIZE_ARG_ONLY
import org.sonarsource.kotlin.api.checks.CallAbstractCheck
import org.sonarsource.kotlin.api.checks.ConstructorMatcher
import org.sonarsource.kotlin.api.checks.FunMatcherImpl
import org.sonarsource.kotlin.api.checks.getFirstArgumentExpression
import org.sonarsource.kotlin.api.checks.isBytesInitializedFromString
import org.sonarsource.kotlin.api.checks.isInitializedPredictably
import org.sonarsource.kotlin.api.checks.matches
import org.sonarsource.kotlin.api.checks.predictRuntimeIntValue
import org.sonarsource.kotlin.api.checks.predictRuntimeValueExpression
import org.sonarsource.kotlin.api.checks.simpleArgExpressionOrNull
import org.sonarsource.kotlin.api.frontend.KotlinFileContext
import org.sonarsource.kotlin.api.frontend.secondaryOf
import org.sonarsource.kotlin.api.visiting.withKaSession

private const val SPECS_PACKAGE = "javax.crypto.spec"
private const val KEY_SPEC_FUN_NAME = "PBEKeySpec"
Expand All @@ -49,31 +49,28 @@ private val matcherSaltIndexMap = mapOf(
ConstructorMatcher("$SPECS_PACKAGE.$PARAMETER_SPEC_FUN_NAME") to 0,
)

@org.sonarsource.kotlin.api.frontend.K1only
@Rule(key = "S2053")
class UnpredictableHashSaltCheck : CallAbstractCheck() {
override val functionsToVisit = matcherSaltIndexMap.keys

override fun visitFunctionCall(
callExpression: KtCallExpression,
resolvedCall: ResolvedCall<*>,
resolvedCall: KaFunctionCall<*>,
matchedFun: FunMatcherImpl,
kotlinFileContext: KotlinFileContext
) {
) = withKaSession {
val saltArgIndex = matcherSaltIndexMap[matchedFun]!!

if (callExpression.valueArguments.size < 2) {
kotlinFileContext.reportIssue(callExpression, MSG_ADD_SALT)
return
}

val bindingContext = kotlinFileContext.bindingContext
val saltArg = resolvedCall.valueArgumentsByIndex?.elementAtOrNull(saltArgIndex)
?.arguments?.elementAtOrNull(0)?.getArgumentExpression() ?: return
val saltArg = resolvedCall.argumentMapping.keys.elementAtOrNull(saltArgIndex) ?: return

val predictedSaltValue = saltArg.predictRuntimeValueExpression(bindingContext)
val predictedSaltValue = saltArg.predictRuntimeValueExpression()

if (predictedSaltValue.isBytesInitializedFromString(bindingContext)) {
if (predictedSaltValue.isBytesInitializedFromString()) {
kotlinFileContext.reportIssue(
saltArg,
MSG_MAKE_UNPREDICTABLE,
Expand All @@ -82,15 +79,15 @@ class UnpredictableHashSaltCheck : CallAbstractCheck() {
return
}

val saltInitializer = predictedSaltValue.getResolvedCall(bindingContext)
val saltInitializer = predictedSaltValue.resolveToCall()?.successfulFunctionCallOrNull()
?.takeIf { it matches BYTE_ARRAY_CONSTRUCTOR } ?: return

if (saltInitializer.byteArrayInitSizeTooSmall(bindingContext)) {
if (saltInitializer.byteArrayInitSizeTooSmall()) {
kotlinFileContext.reportIssue(saltArg, MSG_MIN_LEN, listOf(kotlinFileContext.secondaryOf(predictedSaltValue)))
}

if (saltInitializer matches BYTE_ARRAY_CONSTRUCTOR_SIZE_ARG_ONLY &&
saltArg.isInitializedPredictably(predictedSaltValue, bindingContext)
saltArg.isInitializedPredictably(predictedSaltValue)
) {
kotlinFileContext.reportIssue(
saltArg,
Expand All @@ -103,6 +100,6 @@ class UnpredictableHashSaltCheck : CallAbstractCheck() {
/**
* Checks whether the first argument of the call is an integer that is at least 16
*/
private fun ResolvedCall<*>.byteArrayInitSizeTooSmall(bindingContext: BindingContext) =
(this.simpleArgExpressionOrNull(0)?.predictRuntimeIntValue(bindingContext) ?: 16) < 16
private fun KaFunctionCall<*>.byteArrayInitSizeTooSmall() =
(getFirstArgumentExpression()?.predictRuntimeIntValue() ?: 16) < 16
}

0 comments on commit 5d1d95b

Please sign in to comment.