Partial modification

This commit is contained in:
haby0
2021-12-03 18:59:24 +08:00
parent 6742beae1b
commit 6c6113b85b
7 changed files with 146 additions and 257 deletions

View File

@@ -13,6 +13,7 @@
import java
import DataFlow::PathGraph
import MyBatisCommonLib
import MyBatisAnnotationSqlInjectionLib
import semmle.code.java.dataflow.FlowSources
@@ -32,13 +33,6 @@ private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::
}
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(MethodAccess ma |
ma.getMethod().getDeclaringType() instanceof MapType and
ma.getMethod().getName() = "get" and
ma.getQualifier() = node1.asExpr() and
ma = node2.asExpr()
)
or
exists(MethodAccess ma |
ma.getMethod().getDeclaringType() instanceof TypeObject and
ma.getMethod().getName() = "toString" and
@@ -53,7 +47,7 @@ from
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa
where
cfg.hasFlowPath(source, sink) and
isMybatisAnnotationSqlInjection(sink.getNode(), isoa)
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), _, isoa)
select sink.getNode(), source, sink,
"MyBatis annotation SQL injection might include code from $@ to $@.", source.getNode(),
"this user input", isoa, "this SQL operation"

View File

@@ -4,7 +4,6 @@
import java
import MyBatisCommonLib
import semmle.code.xml.MyBatisMapperXML
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.frameworks.Properties
@@ -16,113 +15,3 @@ class MyBatisAnnotationMethodCallAnArgument extends DataFlow::Node {
)
}
}
/** Get the #{...} or ${...} parameters in the Mybatis annotation value. */
private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) {
result = isoa.getSqlValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
}
predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperationAnnotation isoa) {
// MyBatis uses an annotation method to perform SQL operations. This method has only one parameter and
// the parameter is not annotated with `@Param`. Improper use of this parameter has a SQL injection vulnerability.
// e.g.
//
// ```java
// @Select(select id,name from test where name like '%${value}%')
// Test test(String name);
// ```
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, string res |
msoam = ma.getMethod()
|
msoam.getAnAnnotation() = isoa and
res = getAnMybatiAnnotationSetValue(isoa) and
msoam.getNumberOfParameters() = 1 and
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
res.matches("%${%}") and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getAnArgument() = node.asExpr()
)
or
// MyBatis uses an annotation method to perform SQL operations. The parameter type of the method parameter
// is Map or List or Array. SQL injection vulnerability caused by improper use of this parameter.
// e.g.
//
// ```java
// @Select(select id,name from test where name like '%${value}%')
// Test test(Map map);
// ```
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
msoam = ma.getMethod()
|
msoam.getAnAnnotation() = isoa and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
(
ma.getMethod().getParameterType(i) instanceof MapType or
ma.getMethod().getParameterType(i) instanceof ListType or
ma.getMethod().getParameterType(i) instanceof Array
) and
res = getAnMybatiAnnotationSetValue(isoa) and
res.matches("%${%}") and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
or
// MyBatis uses annotation methods to perform SQL operations, and SQL injection vulnerabilities caused by
// improper use of instance class fields.
// e.g.
//
// ```java
// @Select(select id,name from test order by ${name,jdbcType=VARCHAR})
// void test(Test test);
// ```
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, RefType t |
msoam = ma.getMethod()
|
msoam.getAnAnnotation() = isoa and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
ma.getMethod().getParameterType(i).getName() = t.getName() and
getAnMybatiAnnotationSetValue(isoa).matches("%${" + t.getAField().getName() + "%}") and
ma.getArgument(i) = node.asExpr()
)
or
// MyBatis uses annotations to perform SQL operations. The method parameters use `@Param` annotation.
// Due to improper use of this parameter, SQL injection vulnerabilities are caused.
// e.g.
//
// ```java
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
// void test(@Param("orderby") String name);
// ```
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Annotation annotation |
msoam = ma.getMethod() and ma.getMethod().getParameter(i).getAnAnnotation() = annotation
|
msoam.getAnAnnotation() = isoa and
annotation.getType() instanceof TypeParam and
getAnMybatiAnnotationSetValue(isoa)
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
"%}") and
ma.getArgument(i) = node.asExpr()
)
or
// MyBatis Mapper method default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
// e.g.
//
// ```java
// @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR})
// void test(String name);
// ```
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
msoam = ma.getMethod()
|
msoam.getAnAnnotation() = isoa and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
res = getAnMybatiAnnotationSetValue(isoa) and
(
res.matches("%${param" + (i + 1) + "%}")
or
res.matches("%${arg" + i + "%}")
) and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
}

View File

@@ -3,6 +3,7 @@
*/
import java
import semmle.code.xml.MyBatisMapperXML
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.frameworks.MyBatis
import semmle.code.java.frameworks.Properties
@@ -42,3 +43,136 @@ class ListType extends RefType {
this.getSourceDeclaration().getASourceSupertype*().hasQualifiedName("java.util", "List")
}
}
/** Gets a call to the MyBatis mapper xml method. */
MethodAccess getMyBatisMapperXmlMethodAccess(XMLElement xmle) {
exists(MyBatisMapperSqlOperation mbmxe |
mbmxe.getMapperMethod() = result.getMethod() and
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
)
)
}
/** Gets a call to the MyBatis SQL operation annotation method. */
MethodAccess getMyBatisSqlOperationAnnotationMethodAccess(IbatisSqlOperationAnnotation isoa) {
exists(MyBatisSqlOperationAnnotationMethod msoam |
msoam = result.getMethod() and
msoam.getAnAnnotation() = isoa
)
}
/** Get the #{...} or ${...} parameters in the Mybatis mapper xml file. */
private string getAnMybatiXmlSetValue(XMLElement xmle) {
result = xmle.getTextValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
}
/** Get the #{...} or ${...} parameters in the Mybatis sql operation annotation value. */
private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) {
result = isoa.getSqlValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
}
/** Holds if it is SQL injection of MyBatis xml or MyBatis annotation. */
predicate isMybatisXmlOrAnnotationSqlInjection(
DataFlow::Node node, XMLElement xmle, IbatisSqlOperationAnnotation isoa
) {
exists(MethodAccess ma, string setValue |
(
ma = getMyBatisMapperXmlMethodAccess(xmle) and
setValue = getAnMybatiXmlSetValue(xmle)
or
ma = getMyBatisSqlOperationAnnotationMethodAccess(isoa) and
setValue = getAnMybatiAnnotationSetValue(isoa)
) and
(
// The method parameters use `@Param` annotation. Due to improper use of this parameter, SQL injection vulnerabilities are caused.
// e.g.
//
// ```java
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
// void test(@Param("orderby") String name);
// ```
exists(int i, Annotation annotation |
setValue
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue()
+ "%}") and
annotation.getType() instanceof TypeParam and
ma.getArgument(i) = node.asExpr()
)
or
// MyBatis default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
// e.g.
//
// ```java
// @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR})
// void test(String name);
// ```
exists(int i |
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
(
setValue.matches("%${param" + (i + 1) + "%}")
or
setValue.matches("%${arg" + i + "%}")
) and
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
or
// SQL injection vulnerability caused by improper use of MyBatis instance class fields.
// e.g.
//
// ```java
// @Select(select id,name from test order by ${name,jdbcType=VARCHAR})
// void test(Test test);
// ```
exists(int i, RefType t |
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
ma.getMethod().getParameterType(i).getName() = t.getName() and
setValue.matches("%${" + t.getAField().getName() + "%}") and
ma.getArgument(i) = node.asExpr()
)
or
// The parameter type of the MyBatis method parameter is Map or List or Array.
// SQL injection vulnerability caused by improper use of this parameter.
// e.g.
//
// ```java
// @Select(select id,name from test where name like '%${value}%')
// Test test(Map map);
// ```
exists(int i, MyBatisMapperForeach mbmf |
mbmf = xmle and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
(
ma.getMethod().getParameterType(i) instanceof MapType or
ma.getMethod().getParameterType(i) instanceof ListType or
ma.getMethod().getParameterType(i) instanceof Array
) and
setValue.matches("%${%}") and
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
or
// This method has only one parameter and the parameter is not annotated with `@Param`.
// Improper use of this parameter has a SQL injection vulnerability.
// e.g.
//
// ```java
// @Select(select id,name from test where name like '%${value}%')
// Test test(String name);
// ```
exists(int i | i = 1 |
ma.getMethod().getNumberOfParameters() = i and
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
setValue.matches("%${%}") and
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getAnArgument() = node.asExpr()
)
)
)
}

View File

@@ -3,8 +3,8 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>The MyBatis Mapper XML file allows the use of the $ character to construct dynamic SQL statements.
Attackers can modify the meaning of statements or execute arbitrary SQL commands.</p>
<p>MyBatis allows operating the database by creating XML files to construct dynamic SQL statements.
If the syntax <code>${param}</code> is used in those statements, and <code>param</code> is under the user's control, attackers can exploit this to tamper with the SQL statements or execute arbitrary SQL commands.</p>
</overview>
<<recommendation>
@@ -15,11 +15,11 @@ When writing MyBatis mapping statements, try to use the syntax <code>#{xxx}</cod
<example>
<p>
The following examples show the bad situation and the good situation respectively. In <code>bad1</code>
and <code>bad2</code> and <code>bad3</code> and <code>bad4</code> and <code >bad5</code>, the program
${xxx} are dynamic SQL statements, these five examples of SQL injection vulnerabilities. In <code>good1</code>,
the program uses the ${xxx} dynamic feature SQL statement, but there are subtle restrictions on the data,
and there is no SQL injection vulnerability.
The following sample shows several bad and good examples of MyBatis XML files usage. In <code>bad1</code>,
<code>bad2</code>, <code>bad3</code>, <code>bad4</code>, and <code >bad5</code> the syntax
<code>${xxx}</code> is used to build dynamic SQL statements, which causes a SQL injection vulnerability. In <code>good1</code>,
the program uses the <code>${xxx}</code> syntax, but there are subtle restrictions on the data,
while in <code>good2</code> the syntax <code>#{xxx}</code> is used. In both cases the SQL injection vulnerability is prevented.
</p>
<sample src="MyBatisMapperXmlSqlInjection.xml" />
</example>

View File

@@ -13,6 +13,7 @@
import java
import DataFlow::PathGraph
import MyBatisCommonLib
import MyBatisMapperXmlSqlInjectionLib
import semmle.code.xml.MyBatisMapperXML
import semmle.code.java.dataflow.FlowSources
@@ -33,13 +34,6 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C
}
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(MethodAccess ma |
ma.getMethod().getDeclaringType() instanceof MapType and
ma.getMethod().getName() = "get" and
ma.getQualifier() = node1.asExpr() and
ma = node2.asExpr()
)
or
exists(MethodAccess ma |
ma.getMethod().getDeclaringType() instanceof TypeObject and
ma.getMethod().getName() = "toString" and
@@ -54,7 +48,7 @@ from
XMLElement xmle
where
cfg.hasFlowPath(source, sink) and
isMapperXmlSqlInjection(sink.getNode(), xmle)
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), xmle, _)
select sink.getNode(), source, sink,
"MyBatis Mapper XML SQL injection might include code from $@ to $@.", source.getNode(),
"this user input", xmle, "this SQL operation"

View File

@@ -3,7 +3,6 @@
*/
import java
import MyBatisCommonLib
import semmle.code.xml.MyBatisMapperXML
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.frameworks.Properties
@@ -18,123 +17,3 @@ class MyBatisMapperMethodCallAnArgument extends DataFlow::Node {
)
}
}
/** Get the #{...} or ${...} parameters in the Mybatis mapper xml file */
private string getAnMybatiXmlSetValue(XMLElement xmle) {
result = xmle.getTextValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
}
predicate isMapperXmlSqlInjection(DataFlow::Node node, XMLElement xmle) {
// MyBatis Mapper method Param Annotation sql injection vulnerabilities.
// e.g. MyBatis Mapper method: `void test(@Param("orderby") String name);` and MyBatis Mapper XML file:`select id,name from test order by ${orderby,jdbcType=VARCHAR}`
exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, Annotation annotation |
mbmxe.getMapperMethod() = ma.getMethod()
|
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
) and
ma.getMethod().getParameter(i).getAnAnnotation() = annotation and
annotation.getType() instanceof TypeParam and
getAnMybatiXmlSetValue(xmle)
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
"%}") and
ma.getArgument(i) = node.asExpr()
)
or
// MyBatis Mapper method Class Field sql injection vulnerabilities.
// e.g. MyBatis Mapper method: `void test(Test test);` and MyBatis Mapper XML file:`select id,name from test order by ${name,jdbcType=VARCHAR}`
exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, RefType t |
mbmxe.getMapperMethod() = ma.getMethod()
|
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
) and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
ma.getMethod().getParameterType(i).getName() = t.getName() and
getAnMybatiXmlSetValue(xmle).matches("%${" + t.getAField().getName() + "%}") and
ma.getArgument(i) = node.asExpr()
)
or
// The parameter type of MyBatis Mapper method is Map or List or Array, which may cause SQL injection vulnerability.
// e.g. MyBatis Mapper method: `void test(Map<String, String> params);` and MyBatis Mapper XML file:`select id,name from test where name like '%${name}%'`
exists(
MyBatisMapperSqlOperation mbmxe, MyBatisMapperForeach mbmf, MethodAccess ma, int i, string res
|
mbmxe.getMapperMethod() = ma.getMethod()
|
mbmf = xmle and
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
) and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
(
ma.getMethod().getParameterType(i) instanceof MapType or
ma.getMethod().getParameterType(i) instanceof ListType or
ma.getMethod().getParameterType(i) instanceof Array
) and
res = getAnMybatiXmlSetValue(xmle) and
res.matches("%${%}") and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
or
// SQL injection vulnerability where the MyBatis Mapper method has only one parameter and the parameter is not annotated with `@Param`.
// e.g. MyBatis Mapper method: `void test(String name);` and MyBatis Mapper XML file:`select id,name from test where name like '%${value}%'`
exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, string res |
mbmxe.getMapperMethod() = ma.getMethod()
|
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
) and
res = getAnMybatiXmlSetValue(xmle) and
ma.getMethod().getNumberOfParameters() = 1 and
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
res.matches("%${%}") and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getAnArgument() = node.asExpr()
)
or
// MyBatis Mapper method default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
// e.g. MyBatis Mapper method: `void test(String name);` and MyBatis Mapper XML file:`select id,name from test where name like '%${arg0 or param1}%'`
exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, string res |
mbmxe.getMapperMethod() = ma.getMethod()
|
(
mbmxe.getAChild*() = xmle
or
exists(MyBatisMapperSql mbms |
mbmxe.getInclude().getRefid() = mbms.getId() and
mbms.getAChild*() = xmle
)
) and
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
res = getAnMybatiXmlSetValue(xmle) and
(
res.matches("%${param" + (i + 1) + "%}")
or
res.matches("%${arg" + i + "%}")
) and
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
ma.getArgument(i) = node.asExpr()
)
}

View File

@@ -10,8 +10,7 @@ import java
class MyBatisMapperXMLFile extends XMLFile {
MyBatisMapperXMLFile() {
count(XMLElement e | e = this.getAChild()) = 1 and
this.getAChild().getName() = "mapper" and
this.getFile().getAbsolutePath().indexOf("/src/main") > 0
this.getAChild().getName() = "mapper"
}
}