Skip to content

Commit 6c6113b

Browse files
haby0haby0
committed
Partial modification
1 parent 6742bea commit 6c6113b

File tree

7 files changed

+146
-257
lines changed

7 files changed

+146
-257
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import java
1515
import DataFlow::PathGraph
16+
import MyBatisCommonLib
1617
import MyBatisAnnotationSqlInjectionLib
1718
import semmle.code.java.dataflow.FlowSources
1819

@@ -32,13 +33,6 @@ private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::
3233
}
3334

3435
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
35-
exists(MethodAccess ma |
36-
ma.getMethod().getDeclaringType() instanceof MapType and
37-
ma.getMethod().getName() = "get" and
38-
ma.getQualifier() = node1.asExpr() and
39-
ma = node2.asExpr()
40-
)
41-
or
4236
exists(MethodAccess ma |
4337
ma.getMethod().getDeclaringType() instanceof TypeObject and
4438
ma.getMethod().getName() = "toString" and
@@ -53,7 +47,7 @@ from
5347
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa
5448
where
5549
cfg.hasFlowPath(source, sink) and
56-
isMybatisAnnotationSqlInjection(sink.getNode(), isoa)
50+
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), _, isoa)
5751
select sink.getNode(), source, sink,
5852
"MyBatis annotation SQL injection might include code from $@ to $@.", source.getNode(),
5953
"this user input", isoa, "this SQL operation"

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjectionLib.qll

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import java
66
import MyBatisCommonLib
7-
import semmle.code.xml.MyBatisMapperXML
87
import semmle.code.java.dataflow.FlowSources
98
import semmle.code.java.frameworks.Properties
109

@@ -16,113 +15,3 @@ class MyBatisAnnotationMethodCallAnArgument extends DataFlow::Node {
1615
)
1716
}
1817
}
19-
20-
/** Get the #{...} or ${...} parameters in the Mybatis annotation value. */
21-
private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) {
22-
result = isoa.getSqlValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
23-
}
24-
25-
predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperationAnnotation isoa) {
26-
// MyBatis uses an annotation method to perform SQL operations. This method has only one parameter and
27-
// the parameter is not annotated with `@Param`. Improper use of this parameter has a SQL injection vulnerability.
28-
// e.g.
29-
//
30-
// ```java
31-
// @Select(select id,name from test where name like '%${value}%')
32-
// Test test(String name);
33-
// ```
34-
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, string res |
35-
msoam = ma.getMethod()
36-
|
37-
msoam.getAnAnnotation() = isoa and
38-
res = getAnMybatiAnnotationSetValue(isoa) and
39-
msoam.getNumberOfParameters() = 1 and
40-
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
41-
res.matches("%${%}") and
42-
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
43-
ma.getAnArgument() = node.asExpr()
44-
)
45-
or
46-
// MyBatis uses an annotation method to perform SQL operations. The parameter type of the method parameter
47-
// is Map or List or Array. SQL injection vulnerability caused by improper use of this parameter.
48-
// e.g.
49-
//
50-
// ```java
51-
// @Select(select id,name from test where name like '%${value}%')
52-
// Test test(Map map);
53-
// ```
54-
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
55-
msoam = ma.getMethod()
56-
|
57-
msoam.getAnAnnotation() = isoa and
58-
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
59-
(
60-
ma.getMethod().getParameterType(i) instanceof MapType or
61-
ma.getMethod().getParameterType(i) instanceof ListType or
62-
ma.getMethod().getParameterType(i) instanceof Array
63-
) and
64-
res = getAnMybatiAnnotationSetValue(isoa) and
65-
res.matches("%${%}") and
66-
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
67-
ma.getArgument(i) = node.asExpr()
68-
)
69-
or
70-
// MyBatis uses annotation methods to perform SQL operations, and SQL injection vulnerabilities caused by
71-
// improper use of instance class fields.
72-
// e.g.
73-
//
74-
// ```java
75-
// @Select(select id,name from test order by ${name,jdbcType=VARCHAR})
76-
// void test(Test test);
77-
// ```
78-
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, RefType t |
79-
msoam = ma.getMethod()
80-
|
81-
msoam.getAnAnnotation() = isoa and
82-
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
83-
ma.getMethod().getParameterType(i).getName() = t.getName() and
84-
getAnMybatiAnnotationSetValue(isoa).matches("%${" + t.getAField().getName() + "%}") and
85-
ma.getArgument(i) = node.asExpr()
86-
)
87-
or
88-
// MyBatis uses annotations to perform SQL operations. The method parameters use `@Param` annotation.
89-
// Due to improper use of this parameter, SQL injection vulnerabilities are caused.
90-
// e.g.
91-
//
92-
// ```java
93-
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
94-
// void test(@Param("orderby") String name);
95-
// ```
96-
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Annotation annotation |
97-
msoam = ma.getMethod() and ma.getMethod().getParameter(i).getAnAnnotation() = annotation
98-
|
99-
msoam.getAnAnnotation() = isoa and
100-
annotation.getType() instanceof TypeParam and
101-
getAnMybatiAnnotationSetValue(isoa)
102-
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
103-
"%}") and
104-
ma.getArgument(i) = node.asExpr()
105-
)
106-
or
107-
// MyBatis Mapper method default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
108-
// e.g.
109-
//
110-
// ```java
111-
// @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR})
112-
// void test(String name);
113-
// ```
114-
exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
115-
msoam = ma.getMethod()
116-
|
117-
msoam.getAnAnnotation() = isoa and
118-
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
119-
res = getAnMybatiAnnotationSetValue(isoa) and
120-
(
121-
res.matches("%${param" + (i + 1) + "%}")
122-
or
123-
res.matches("%${arg" + i + "%}")
124-
) and
125-
not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and
126-
ma.getArgument(i) = node.asExpr()
127-
)
128-
}

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import java
6+
import semmle.code.xml.MyBatisMapperXML
67
import semmle.code.java.dataflow.FlowSources
78
import semmle.code.java.frameworks.MyBatis
89
import semmle.code.java.frameworks.Properties
@@ -42,3 +43,136 @@ class ListType extends RefType {
4243
this.getSourceDeclaration().getASourceSupertype*().hasQualifiedName("java.util", "List")
4344
}
4445
}
46+
47+
/** Gets a call to the MyBatis mapper xml method. */
48+
MethodAccess getMyBatisMapperXmlMethodAccess(XMLElement xmle) {
49+
exists(MyBatisMapperSqlOperation mbmxe |
50+
mbmxe.getMapperMethod() = result.getMethod() and
51+
(
52+
mbmxe.getAChild*() = xmle
53+
or
54+
exists(MyBatisMapperSql mbms |
55+
mbmxe.getInclude().getRefid() = mbms.getId() and
56+
mbms.getAChild*() = xmle
57+
)
58+
)
59+
)
60+
}
61+
62+
/** Gets a call to the MyBatis SQL operation annotation method. */
63+
MethodAccess getMyBatisSqlOperationAnnotationMethodAccess(IbatisSqlOperationAnnotation isoa) {
64+
exists(MyBatisSqlOperationAnnotationMethod msoam |
65+
msoam = result.getMethod() and
66+
msoam.getAnAnnotation() = isoa
67+
)
68+
}
69+
70+
/** Get the #{...} or ${...} parameters in the Mybatis mapper xml file. */
71+
private string getAnMybatiXmlSetValue(XMLElement xmle) {
72+
result = xmle.getTextValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
73+
}
74+
75+
/** Get the #{...} or ${...} parameters in the Mybatis sql operation annotation value. */
76+
private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) {
77+
result = isoa.getSqlValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
78+
}
79+
80+
/** Holds if it is SQL injection of MyBatis xml or MyBatis annotation. */
81+
predicate isMybatisXmlOrAnnotationSqlInjection(
82+
DataFlow::Node node, XMLElement xmle, IbatisSqlOperationAnnotation isoa
83+
) {
84+
exists(MethodAccess ma, string setValue |
85+
(
86+
ma = getMyBatisMapperXmlMethodAccess(xmle) and
87+
setValue = getAnMybatiXmlSetValue(xmle)
88+
or
89+
ma = getMyBatisSqlOperationAnnotationMethodAccess(isoa) and
90+
setValue = getAnMybatiAnnotationSetValue(isoa)
91+
) and
92+
(
93+
// The method parameters use `@Param` annotation. Due to improper use of this parameter, SQL injection vulnerabilities are caused.
94+
// e.g.
95+
//
96+
// ```java
97+
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
98+
// void test(@Param("orderby") String name);
99+
// ```
100+
exists(int i, Annotation annotation |
101+
setValue
102+
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue()
103+
+ "%}") and
104+
annotation.getType() instanceof TypeParam and
105+
ma.getArgument(i) = node.asExpr()
106+
)
107+
or
108+
// MyBatis default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
109+
// e.g.
110+
//
111+
// ```java
112+
// @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR})
113+
// void test(String name);
114+
// ```
115+
exists(int i |
116+
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
117+
(
118+
setValue.matches("%${param" + (i + 1) + "%}")
119+
or
120+
setValue.matches("%${arg" + i + "%}")
121+
) and
122+
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
123+
ma.getArgument(i) = node.asExpr()
124+
)
125+
or
126+
// SQL injection vulnerability caused by improper use of MyBatis instance class fields.
127+
// e.g.
128+
//
129+
// ```java
130+
// @Select(select id,name from test order by ${name,jdbcType=VARCHAR})
131+
// void test(Test test);
132+
// ```
133+
exists(int i, RefType t |
134+
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
135+
ma.getMethod().getParameterType(i).getName() = t.getName() and
136+
setValue.matches("%${" + t.getAField().getName() + "%}") and
137+
ma.getArgument(i) = node.asExpr()
138+
)
139+
or
140+
// The parameter type of the MyBatis method parameter is Map or List or Array.
141+
// SQL injection vulnerability caused by improper use of this parameter.
142+
// e.g.
143+
//
144+
// ```java
145+
// @Select(select id,name from test where name like '%${value}%')
146+
// Test test(Map map);
147+
// ```
148+
exists(int i, MyBatisMapperForeach mbmf |
149+
mbmf = xmle and
150+
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
151+
(
152+
ma.getMethod().getParameterType(i) instanceof MapType or
153+
ma.getMethod().getParameterType(i) instanceof ListType or
154+
ma.getMethod().getParameterType(i) instanceof Array
155+
) and
156+
setValue.matches("%${%}") and
157+
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
158+
ma.getArgument(i) = node.asExpr()
159+
)
160+
or
161+
// This method has only one parameter and the parameter is not annotated with `@Param`.
162+
// Improper use of this parameter has a SQL injection vulnerability.
163+
// e.g.
164+
//
165+
// ```java
166+
// @Select(select id,name from test where name like '%${value}%')
167+
// Test test(String name);
168+
// ```
169+
exists(int i | i = 1 |
170+
ma.getMethod().getNumberOfParameters() = i and
171+
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
172+
setValue.matches("%${%}") and
173+
not setValue = "${" + getAnMybatisConfigurationVariableKey() + "}" and
174+
ma.getAnArgument() = node.asExpr()
175+
)
176+
)
177+
)
178+
}

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.qhelp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"qhelp.dtd">
44
<qhelp>
55
<overview>
6-
<p>The MyBatis Mapper XML file allows the use of the $ character to construct dynamic SQL statements.
7-
Attackers can modify the meaning of statements or execute arbitrary SQL commands.</p>
6+
<p>MyBatis allows operating the database by creating XML files to construct dynamic SQL statements.
7+
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>
88
</overview>
99

1010
<<recommendation>
@@ -15,11 +15,11 @@ When writing MyBatis mapping statements, try to use the syntax <code>#{xxx}</cod
1515

1616
<example>
1717
<p>
18-
The following examples show the bad situation and the good situation respectively. In <code>bad1</code>
19-
and <code>bad2</code> and <code>bad3</code> and <code>bad4</code> and <code >bad5</code>, the program
20-
${xxx} are dynamic SQL statements, these five examples of SQL injection vulnerabilities. In <code>good1</code>,
21-
the program uses the ${xxx} dynamic feature SQL statement, but there are subtle restrictions on the data,
22-
and there is no SQL injection vulnerability.
18+
The following sample shows several bad and good examples of MyBatis XML files usage. In <code>bad1</code>,
19+
<code>bad2</code>, <code>bad3</code>, <code>bad4</code>, and <code >bad5</code> the syntax
20+
<code>${xxx}</code> is used to build dynamic SQL statements, which causes a SQL injection vulnerability. In <code>good1</code>,
21+
the program uses the <code>${xxx}</code> syntax, but there are subtle restrictions on the data,
22+
while in <code>good2</code> the syntax <code>#{xxx}</code> is used. In both cases the SQL injection vulnerability is prevented.
2323
</p>
2424
<sample src="MyBatisMapperXmlSqlInjection.xml" />
2525
</example>

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import java
1515
import DataFlow::PathGraph
16+
import MyBatisCommonLib
1617
import MyBatisMapperXmlSqlInjectionLib
1718
import semmle.code.xml.MyBatisMapperXML
1819
import semmle.code.java.dataflow.FlowSources
@@ -33,13 +34,6 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C
3334
}
3435

3536
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
36-
exists(MethodAccess ma |
37-
ma.getMethod().getDeclaringType() instanceof MapType and
38-
ma.getMethod().getName() = "get" and
39-
ma.getQualifier() = node1.asExpr() and
40-
ma = node2.asExpr()
41-
)
42-
or
4337
exists(MethodAccess ma |
4438
ma.getMethod().getDeclaringType() instanceof TypeObject and
4539
ma.getMethod().getName() = "toString" and
@@ -54,7 +48,7 @@ from
5448
XMLElement xmle
5549
where
5650
cfg.hasFlowPath(source, sink) and
57-
isMapperXmlSqlInjection(sink.getNode(), xmle)
51+
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), xmle, _)
5852
select sink.getNode(), source, sink,
5953
"MyBatis Mapper XML SQL injection might include code from $@ to $@.", source.getNode(),
6054
"this user input", xmle, "this SQL operation"

0 commit comments

Comments
 (0)