Skip to content

Commit

Permalink
feat: Add injection filter detection rules
Browse files Browse the repository at this point in the history
Find possible injection filters by looking for keywords (SQLFilter,XSSFilter, ClearXSS, etc.).
  • Loading branch information
springkill committed Nov 3, 2024
1 parent f44692a commit ff988b4
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,79 @@
package org.skgroup.securityinspector.rules.filters

class InjectionFilter {
import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.JavaElementVisitor
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiMethod
import com.intellij.psi.PsiMethodCallExpression
import org.skgroup.securityinspector.inspectors.BaseLocalInspectionTool
import org.skgroup.securityinspector.utils.InspectionBundle
import org.skgroup.securityinspector.utils.SecExpressionUtils

class InjectionFilter : BaseLocalInspectionTool() {

companion object {
private val SQLFILTER_MESSAGE = InspectionBundle.message("vuln.massage.SQLFilter")
private val XSSFILTER_MESSAGE = InspectionBundle.message("vuln.massage.XSSFilter")
private val MAYBE_SQL_FILTER_NAME = listOf(
"SQLFilter", "SQLInjectionFilter", "SQLInjection"
)
private val MAYBE_XSS_FILTER_NAME = listOf(
"XSSFilter", "XSSClear", "ClearXSS"
)
private val MAYBE_SQL_FILTER_METHODS = listOf(
"ClearSQL", "SQLClear"
)
private val MAYBE_XSS_FILTER_METHODS = listOf(
"ClearXSS", "XSSClear"
)
}

override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return object : JavaElementVisitor() {
override fun visitClass(aClass: PsiClass) {
MAYBE_SQL_FILTER_NAME.forEach {
if (SecExpressionUtils.matchesClassName(aClass, it)) {
holder.registerProblem(
aClass,
SQLFILTER_MESSAGE,
ProblemHighlightType.GENERIC_ERROR_OR_WARNING
)
}
}
MAYBE_XSS_FILTER_NAME.forEach {
if (SecExpressionUtils.matchesClassName(aClass, it)) {
holder.registerProblem(
aClass,
XSSFILTER_MESSAGE,
ProblemHighlightType.GENERIC_ERROR_OR_WARNING
)
}
}
}

//需要检查的是定义点而不是调用点,直接用methodName防止warning位置不对
override fun visitMethod(methodName: PsiMethod) {
MAYBE_SQL_FILTER_METHODS.forEach {
if (SecExpressionUtils.matchesMethodName(methodName, it)) {
holder.registerProblem(
methodName,
SQLFILTER_MESSAGE,
ProblemHighlightType.GENERIC_ERROR_OR_WARNING
)
}
}
MAYBE_XSS_FILTER_METHODS.forEach {
if (SecExpressionUtils.matchesMethodName(methodName, it)) {
holder.registerProblem(
methodName,
XSSFILTER_MESSAGE,
ProblemHighlightType.GENERIC_ERROR_OR_WARNING
)
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class JNDIInjection : BaseLocalInspectionTool() {
private val MESSAGE = InspectionBundle.message("vuln.massage.JNDIInjection")

private val JNDIINJECTION_METHOS_SINKS = mapOf(
"javax.management.remote.JMXServiceURL" to emptyList(),
"java.rmi.registry.Registry" to listOf("lookup"),
"javax.naming.Context" to listOf("lookup", "list", "listBindings", "lookupLink", "rename"),
"javax.naming.InitialContext" to listOf("doLookup", "lookup", "rename", "list", "listBindings"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ object SecExpressionUtils {

private val SQLiCareTypeStr = mutableSetOf("java.lang.String", "java.lang.StringBuilder", "java.lang.StringBuffer")

fun resolveField( expression: PsiExpression?): PsiField? {
fun resolveField(expression: PsiExpression?): PsiField? {
var expression = PsiUtil.skipParenthesizedExprDown(expression)
val referenceExpression = ObjectUtils.tryCast(expression, PsiReferenceExpression::class.java)
return referenceExpression?.let { ObjectUtils.tryCast(it.resolve(), PsiField::class.java) }
}

fun getLiteralInnerText( expression: PsiExpression?): String? {
fun getLiteralInnerText(expression: PsiExpression?): String? {
val literal = ExpressionUtils.getLiteral(expression)
return literal?.value?.toString()
}
Expand All @@ -29,7 +29,7 @@ object SecExpressionUtils {
return getText(expression, false)
}

fun getText( expression: PsiExpression?, force: Boolean): String? {
fun getText(expression: PsiExpression?, force: Boolean): String? {
if (expression == null) return null

var value = getLiteralInnerText(expression)
Expand Down Expand Up @@ -69,7 +69,7 @@ object SecExpressionUtils {
return value
}

fun isText( expression: PsiExpression): Boolean {
fun isText(expression: PsiExpression): Boolean {
return getText(expression) != null
}

Expand Down Expand Up @@ -359,10 +359,18 @@ object SecExpressionUtils {
}
}

fun isNewExpressionSink(expression: PsiNewExpression,newExpressionSinks:List<String>): Boolean {
fun isNewExpressionSink(expression: PsiNewExpression, newExpressionSinks: List<String>): Boolean {
return newExpressionSinks.any { className ->
hasFullQualifiedName(expression, className)
}
}

fun matchesClassName(psiClass: PsiClass, pattern: String): Boolean {
return psiClass.name?.contains(pattern, true) == true
}

fun matchesMethodName(psiMethod: PsiMethod, pattern: String): Boolean {
return psiMethod.name.contains(pattern, true)
}

}
3 changes: 3 additions & 0 deletions src/main/resources/InspectionBundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ vuln.name.SystemEXITDOS=System EXIT DOS Risk
vuln.name.ReadFile=Arbitrary File Read Risk
vuln.name.CommonIOFileWrite=CommonIO File Write Risk
vuln.name.IOFilesWrite=Arbitrary File Write Risk
vuln.name.InjectionFilter=Maybe injection filter class
vuln.name.JDBCAttack=JDBC Attack Risk
vuln.name.JNDIInjection=JNDI Injection Risk
vuln.name.LDAPUnserialize=LDAP Unserialize Risk
Expand Down Expand Up @@ -73,6 +74,8 @@ vuln.massage.SystemEXITDOS=Please check for System EXIT DOS Risk
vuln.massage.ReadFile=Please check for Arbitrary File Read Risk
vuln.massage.CommonIOFileWrite=Please check for CommonIO File Write Risk
vuln.massage.IOFilesWrite=Please check for Arbitrary File Write Risk
vuln.massage.SQLFilter=Maybe SQL filter class
vuln.massage.XSSFilter=Maybe XSS filter class
vuln.massage.JDBCAttack=Please check for JDBC Attack Risk
vuln.massage.JNDIInjection=Please check for JNDI Injection Risk
vuln.massage.LDAPUnserialize=Please check for LDAP Unserialize Risk
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/InspectionBundle_zh.properties

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@
bundle="InspectionBundle" key="vuln.name.IOFilesWrite"
implementationClass="org.skgroup.securityinspector.rules.files.write.IOFiles"
/>
<localInspection
language="JAVA" groupPath="Security"
groupName="SecurityInspector" enabledByDefault="true" level="WARNING"
bundle="InspectionBundle" key="vuln.name.InjectionFilter"
implementationClass="org.skgroup.securityinspector.rules.filters.InjectionFilter"
/>
<localInspection
language="JAVA" groupPath="Security"
groupName="SecurityInspector" enabledByDefault="true" level="WARNING"
Expand Down
51 changes: 51 additions & 0 deletions src/main/resources/inspectionDescriptions/InjectionFilter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<html>
<head>
<title>注入过滤器检查项</title>
</head>
<body>
<h1>注入过滤器检查项</h1>
<h2>漏洞类型</h2>
<p>
<strong>SQL 注入</strong><strong>跨站脚本(XSS)注入</strong>是常见的安全漏洞。SQL 注入会导致攻击者可以操控 SQL 查询,进而访问、修改或删除数据库数据。而 XSS 注入则允许攻击者在页面中插入恶意脚本,从而窃取用户信息或劫持会话。
</p>
<h2>检查了什么内容</h2>
<p>
该条目检查了类名和方法名中是否存在可能被误认为是 SQLXSS 过滤器的定义。某些命名(例如 <code>SQLFilter</code><code>ClearXSS</code>)可能误导开发人员,给人以过滤功能的假象,但实际上这些方法或类可能并不具备有效的安全过滤功能,从而产生安全风险。
</p>
<h3>检查逻辑</h3>
<ul>
<li>如果类名与 <code>SQLFilter</code><code>SQLInjectionFilter</code><code>SQLInjection</code> 等模式匹配,则认为该类可能不安全,显示相应的 SQL 注入警告。</li>
<li>如果类名包含 <code>XSSFilter</code><code>XSSClear</code><code>ClearXSS</code>,则会提示可能存在 XSS 安全风险。</li>
<li>同样地,如果方法名匹配 <code>ClearSQL</code><code>SQLClear</code><code>ClearXSS</code><code>XSSClear</code>,则认为该方法可能涉及不安全的过滤操作,显示相应的警告。</li>
</ul>
<h2>修复建议</h2>
<p>
对于 SQLXSS 过滤器,建议确保过滤器功能实际有效,并避免在命名上造成误导。推荐使用成熟的防注入框架或库(例如使用正则表达式、编码函数等),以确保数据的输入有效过滤,防止攻击者进行注入攻击。
</p>
<h3>快速修复</h3>
<p>
该条目会提醒开发者检查这些类和方法的实现,确保它们具备真正的 SQLXSS 防护能力,而不是仅具备误导性的命名。
</p>
<h2>相关示例</h2>
<p>
例如,代码中有以下不安全的类或方法命名:
</p>
<pre>
<code>
class SQLFilter { ... } // 可能不安全
void ClearXSS(String input) { ... } // 可能不安全
</code>
</pre>
<p>
建议确保这些类或方法实现真正的过滤功能,或者使用更具描述性的命名。
</p>
<h2>参考资料</h2>
<p>
了解更多关于 SQLXSS 注入的风险和修复方法,可以参考以下资源:
<ul>
<li><a href="https://owasp.org/www-community/attacks/SQL_Injection">OWASP: SQL Injection</a></li>
<li><a href="https://owasp.org/www-community/attacks/xss/">OWASP: Cross Site Scripting (XSS)</a></li>
</ul>
</p>
</body>
</html>
51 changes: 51 additions & 0 deletions src/main/resources/inspectionDescriptions_en/InjectionFilter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<html>
<head>
<title>Injection Filter Inspection Item</title>
</head>
<body>
<h1>Injection Filter Inspection Item</h1>
<h2>Vulnerability Type</h2>
<p>
<strong>SQL Injection</strong> and <strong>Cross-Site Scripting (XSS) Injection</strong> are common security vulnerabilities. SQL injection can allow attackers to manipulate SQL queries, potentially giving them access to or control over database data. XSS injection allows attackers to insert malicious scripts, which may steal user information or hijack sessions.
</p>
<h2>What This Item Checks</h2>
<p>
This item checks for class and method names that may be misleadingly perceived as SQL or XSS filters. Certain names, such as <code>SQLFilter</code> or <code>ClearXSS</code>, may give a false sense of filtering security, while these methods or classes may lack actual filtering functionality, creating potential security risks.
</p>
<h3>Inspection Logic</h3>
<ul>
<li>If a class name matches patterns like <code>SQLFilter</code>, <code>SQLInjectionFilter</code>, or <code>SQLInjection</code>, it flags the class as potentially unsafe and displays a warning for SQL injection risk.</li>
<li>If a class name contains <code>XSSFilter</code>, <code>XSSClear</code>, or <code>ClearXSS</code>, it indicates potential XSS security risks.</li>
<li>Similarly, if a method name matches <code>ClearSQL</code>, <code>SQLClear</code>, <code>ClearXSS</code>, or <code>XSSClear</code>, it considers the method potentially unsafe and highlights it accordingly.</li>
</ul>
<h2>Fix Recommendation</h2>
<p>
For SQL and XSS filters, it is recommended to ensure that the filtering functionality is genuinely effective and avoids misleading naming. Consider using established anti-injection frameworks or libraries (e.g., regular expressions, encoding functions) to properly validate data inputs and prevent injection attacks.
</p>
<h3>Quick Fix</h3>
<p>
This item reminds developers to review the implementations of these classes and methods to ensure they have real SQL or XSS protection capabilities rather than merely misleading names.
</p>
<h2>Example</h2>
<p>
For example, the following code includes potentially unsafe class or method names:
</p>
<pre>
<code>
class SQLFilter { ... } // Potentially unsafe
void ClearXSS(String input) { ... } // Potentially unsafe
</code>
</pre>
<p>
It is recommended to ensure these classes or methods perform real filtering functions or to use more descriptive names.
</p>
<h2>References</h2>
<p>
To learn more about SQL and XSS injection risks and how to address them, you can refer to the following resources:
<ul>
<li><a href="https://owasp.org/www-community/attacks/SQL_Injection">OWASP: SQL Injection</a></li>
<li><a href="https://owasp.org/www-community/attacks/xss/">OWASP: Cross Site Scripting (XSS)</a></li>
</ul>
</p>
</body>
</html>

0 comments on commit ff988b4

Please sign in to comment.