Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SONARKT-460 Migrate ServerCertificateCheck to kotlin-analysis-api #505

Merged
merged 1 commit into from
Feb 3, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,17 @@
package org.sonarsource.kotlin.checks

import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.js.descriptorUtils.getKotlinTypeFqName
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCatchClause
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtThrowExpression
import org.jetbrains.kotlin.psi.KtVisitorVoid
import org.jetbrains.kotlin.resolve.BindingContext
import org.sonar.check.Rule
import org.sonarsource.kotlin.api.checks.AbstractCheck
import org.sonarsource.kotlin.api.checks.FunMatcher
import org.sonarsource.kotlin.api.checks.determineType
import org.sonarsource.kotlin.api.frontend.KotlinFileContext


private const val CERTIFICATE_EXCEPTION = "java.security.cert.CertificateException"
import org.sonarsource.kotlin.api.visiting.withKaSession

private val funMatchers = listOf(
FunMatcher {
Expand All @@ -43,34 +39,31 @@ private val funMatchers = listOf(
withNames("checkClientTrusted", "checkServerTrusted")
})


@org.sonarsource.kotlin.api.frontend.K1only
@Rule(key = "S4830")
class ServerCertificateCheck : AbstractCheck() {
override fun visitNamedFunction(function: KtNamedFunction, kotlinFileContext: KotlinFileContext) {
val (_, _, bindingContext) = kotlinFileContext

if (function.belongsToTrustManagerClass(bindingContext)
&& !function.callsCheckTrusted(bindingContext)
&& !function.throwsCertificateExceptionWithoutCatching(bindingContext)
override fun visitNamedFunction(function: KtNamedFunction, kotlinFileContext: KotlinFileContext) {
if (function.belongsToTrustManagerClass()
&& !function.callsCheckTrusted()
&& !function.throwsCertificateExceptionWithoutCatching()
) {
kotlinFileContext.reportIssue(function.nameIdentifier ?: function,
"Enable server certificate validation on this SSL/TLS connection.")
}
}

private fun KtNamedFunction.belongsToTrustManagerClass(bindingContext: BindingContext): Boolean =
funMatchers.any { it.matches(this, bindingContext) }
private fun KtNamedFunction.belongsToTrustManagerClass(): Boolean =
funMatchers.any { it.matches(this) }

/*
* Returns true if a function contains a call to "checkClientTrusted" or "checkServerTrusted".
*/
private fun KtNamedFunction.callsCheckTrusted(bindingContext: BindingContext): Boolean {
private fun KtNamedFunction.callsCheckTrusted(): Boolean {
val visitor = object : KtVisitorVoid() {
private var foundCheckTrustedCall: Boolean = false

override fun visitCallExpression(expression: KtCallExpression) {
foundCheckTrustedCall = foundCheckTrustedCall || funMatchers.any { it.matches(expression, bindingContext) }
foundCheckTrustedCall = foundCheckTrustedCall || funMatchers.any { it.matches(expression) }
}

fun callsCheckTrusted(): Boolean = foundCheckTrustedCall
Expand All @@ -82,24 +75,28 @@ class ServerCertificateCheck : AbstractCheck() {
/*
* Returns true only when the function throws a CertificateException without a catch against it.
*/
private fun KtNamedFunction.throwsCertificateExceptionWithoutCatching(bindingContext: BindingContext): Boolean {
val visitor = ThrowCatchVisitor(bindingContext)
private fun KtNamedFunction.throwsCertificateExceptionWithoutCatching(): Boolean {
val visitor = ThrowCatchVisitor()
this.acceptRecursively(visitor)
return visitor.throwsCertificateExceptionWithoutCatching()
}

private class ThrowCatchVisitor(private val bindingContext: BindingContext) : KtVisitorVoid() {
private class ThrowCatchVisitor : KtVisitorVoid() {
private val certificateExceptionClassId = ClassId.fromString("java/security/cert/CertificateException")

private var throwFound: Boolean = false
private var catchFound: Boolean = false

override fun visitThrowExpression(expression: KtThrowExpression) {
override fun visitThrowExpression(expression: KtThrowExpression) = withKaSession {
throwFound =
throwFound || CERTIFICATE_EXCEPTION == expression.thrownExpression.determineType(bindingContext)?.getKotlinTypeFqName(false)
throwFound ||
expression.thrownExpression?.expressionType?.isClassType(certificateExceptionClassId) == true
}

override fun visitCatchSection(catchClause: KtCatchClause) {
override fun visitCatchSection(catchClause: KtCatchClause) = withKaSession {
catchFound =
catchFound || CERTIFICATE_EXCEPTION == catchClause.catchParameter.determineType(bindingContext)?.getKotlinTypeFqName(false)
catchFound ||
catchClause.catchParameter?.symbol?.returnType?.isClassType(certificateExceptionClassId) == true
}

fun throwsCertificateExceptionWithoutCatching(): Boolean {
Expand Down