Skip to content

Commit a18aad8

Browse files
haby0haby0
committed
Fix one
1 parent 1d321c6 commit a18aad8

File tree

3 files changed

+86
-40
lines changed

3 files changed

+86
-40
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,17 @@ private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::
4444

4545
from
4646
MyBatisAnnotationSqlInjectionConfiguration cfg, DataFlow::PathNode source,
47-
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa, MethodAccess ma
47+
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa, MethodAccess ma,
48+
string unsafeExpression
4849
where
4950
cfg.hasFlowPath(source, sink) and
5051
ma.getAnArgument() = sink.getNode().asExpr() and
5152
myBatisSqlOperationAnnotationFromMethod(ma.getMethod(), isoa) and
52-
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, getAMybatisAnnotationSqlValue(isoa), _)
53+
unsafeExpression = getAMybatisAnnotationSqlValue(isoa) and
54+
(
55+
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, unsafeExpression) or
56+
isMybatisAnnotationCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression)
57+
)
5358
select sink.getNode(), source, sink,
5459
"MyBatis annotation SQL injection might include code from $@ to $@.", source.getNode(),
5560
"this user input", isoa, "this SQL operation"

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

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class ListType extends RefType {
4444
}
4545
}
4646

47-
/** Holds if the specified method uses MyBatis Mapper XMLElement. */
47+
/** Holds if the specified `method` uses MyBatis Mapper XMLElement `mmxx`. */
4848
predicate myBatisMapperXMLElementFromMethod(Method method, MyBatisMapperXMLElement mmxx) {
4949
exists(MyBatisMapperSqlOperation mbmxe | mbmxe.getMapperMethod() = method |
5050
mbmxe.getAChild*() = mmxx
@@ -56,31 +56,88 @@ predicate myBatisMapperXMLElementFromMethod(Method method, MyBatisMapperXMLEleme
5656
)
5757
}
5858

59-
/** Holds if the specified method uses Ibatis Sql Operation Annotation. */
59+
/** Holds if the specified `method` has Ibatis Sql operation annotation `isoa`. */
6060
predicate myBatisSqlOperationAnnotationFromMethod(Method method, IbatisSqlOperationAnnotation isoa) {
6161
exists(MyBatisSqlOperationAnnotationMethod msoam |
6262
msoam = method and
6363
msoam.getAnAnnotation() = isoa
6464
)
6565
}
6666

67-
/** Get the #{...} or ${...} parameters in the Mybatis mapper xml file. */
67+
/** Gets a `#{...}` or `${...}` expression argument in XML element `xmle`. */
6868
string getAMybatisXmlSetValue(XMLElement xmle) {
6969
result = xmle.getTextValue().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
7070
}
7171

72-
/** Get the #{...} or ${...} parameters in the Mybatis sql operation annotation value. */
72+
/** Gets a `#{...}` or `${...}` expression argument in annotation `isoa`. */
7373
string getAMybatisAnnotationSqlValue(IbatisSqlOperationAnnotation isoa) {
7474
result = isoa.getSqlValue().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _)
7575
}
7676

77-
/** Holds if it is SQL injection of MyBatis xml or MyBatis annotation. */
78-
bindingset[setValue]
77+
/** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
78+
bindingset[unsafeExpression]
79+
predicate isMybatisAnnotationCollectionTypeSqlInjection(
80+
DataFlow::Node node, MethodAccess ma, string unsafeExpression
81+
) {
82+
not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
83+
// The parameter type of the MyBatis method parameter is Map or List or Array.
84+
// SQL injection vulnerability caused by improper use of this parameter.
85+
// e.g.
86+
//
87+
// ```java
88+
// @Select(select id,name from test where name like '%${value}%')
89+
// Test test(Map map);
90+
// ```
91+
exists(int i |
92+
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
93+
(
94+
ma.getMethod().getParameterType(i) instanceof MapType or
95+
ma.getMethod().getParameterType(i) instanceof ListType or
96+
ma.getMethod().getParameterType(i) instanceof Array
97+
) and
98+
unsafeExpression.matches("${%}") and
99+
ma.getArgument(i) = node.asExpr()
100+
)
101+
}
102+
103+
/** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
104+
bindingset[unsafeExpression]
105+
predicate isMybatisXmlCollectionTypeSqlInjection(
106+
DataFlow::Node node, MethodAccess ma, string unsafeExpression, MyBatisMapperXMLElement mmxe
107+
) {
108+
not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
109+
// The parameter type of the MyBatis method parameter is Map or List or Array.
110+
// SQL injection vulnerability caused by improper use of this parameter.
111+
// e.g.
112+
//
113+
// ```java
114+
// Test test(Map map);
115+
// <select id="test" resultMap="BaseResultMap">
116+
// select id,name from test where name in
117+
// <foreach collection="list" item="value" open="(" close=")" separator=",">
118+
// ${value}
119+
// </foreach>
120+
// </select>
121+
// ```
122+
exists(int i, MyBatisMapperForeach mbmf |
123+
mbmf = mmxe and
124+
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
125+
(
126+
ma.getMethod().getParameterType(i) instanceof MapType or
127+
ma.getMethod().getParameterType(i) instanceof ListType or
128+
ma.getMethod().getParameterType(i) instanceof Array
129+
) and
130+
unsafeExpression.matches("${%}") and
131+
ma.getArgument(i) = node.asExpr()
132+
)
133+
}
134+
135+
/** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
136+
bindingset[unsafeExpression]
79137
predicate isMybatisXmlOrAnnotationSqlInjection(
80-
//DataFlow::Node node, MyBatisMapperXMLElement xmle, IbatisSqlOperationAnnotation isoa
81-
DataFlow::Node node, MethodAccess ma, string setValue, MyBatisMapperXMLElement mmxe
138+
DataFlow::Node node, MethodAccess ma, string unsafeExpression
82139
) {
83-
not setValue.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
140+
not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
84141
(
85142
// The method parameters use `@Param` annotation. Due to improper use of this parameter, SQL injection vulnerabilities are caused.
86143
// e.g.
@@ -89,12 +146,12 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
89146
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
90147
// void test(@Param("orderby") String name);
91148
// ```
92-
exists(int i, Annotation annotation |
93-
setValue
149+
exists(Annotation annotation |
150+
unsafeExpression
94151
.matches("${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
95152
"%}") and
96153
annotation.getType() instanceof TypeParam and
97-
ma.getArgument(i) = node.asExpr()
154+
ma.getAnArgument() = node.asExpr()
98155
)
99156
or
100157
// MyBatis default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
@@ -107,9 +164,9 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
107164
exists(int i |
108165
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
109166
(
110-
setValue.matches("${param" + (i + 1) + "%}")
167+
unsafeExpression.matches("${param" + (i + 1) + "%}")
111168
or
112-
setValue.matches("${arg" + i + "%}")
169+
unsafeExpression.matches("${arg" + i + "%}")
113170
) and
114171
ma.getArgument(i) = node.asExpr()
115172
)
@@ -124,27 +181,7 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
124181
exists(int i, RefType t |
125182
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
126183
ma.getMethod().getParameterType(i).getName() = t.getName() and
127-
setValue.matches("${" + t.getAField().getName() + "%}") and
128-
ma.getArgument(i) = node.asExpr()
129-
)
130-
or
131-
// The parameter type of the MyBatis method parameter is Map or List or Array.
132-
// SQL injection vulnerability caused by improper use of this parameter.
133-
// e.g.
134-
//
135-
// ```java
136-
// @Select(select id,name from test where name like '%${value}%')
137-
// Test test(Map map);
138-
// ```
139-
exists(int i, MyBatisMapperForeach mbmf |
140-
mbmf = mmxe and
141-
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
142-
(
143-
ma.getMethod().getParameterType(i) instanceof MapType or
144-
ma.getMethod().getParameterType(i) instanceof ListType or
145-
ma.getMethod().getParameterType(i) instanceof Array
146-
) and
147-
setValue.matches("${%}") and
184+
unsafeExpression.matches("${" + t.getAField().getName() + "%}") and
148185
ma.getArgument(i) = node.asExpr()
149186
)
150187
or
@@ -160,7 +197,7 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
160197
exists(int i | i = 1 |
161198
ma.getMethod().getNumberOfParameters() = i and
162199
not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and
163-
setValue.matches("${%}") and
200+
unsafeExpression.matches("${%}") and
164201
ma.getAnArgument() = node.asExpr()
165202
)
166203
)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,16 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C
4545

4646
from
4747
MyBatisMapperXmlSqlInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
48-
MyBatisMapperXMLElement mmxe, MethodAccess ma
48+
MyBatisMapperXMLElement mmxe, MethodAccess ma, string unsafeExpression
4949
where
5050
cfg.hasFlowPath(source, sink) and
5151
ma.getAnArgument() = sink.getNode().asExpr() and
5252
myBatisMapperXMLElementFromMethod(ma.getMethod(), mmxe) and
53-
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, getAMybatisXmlSetValue(mmxe), mmxe)
53+
unsafeExpression = getAMybatisXmlSetValue(mmxe) and
54+
(
55+
isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, unsafeExpression) or
56+
isMybatisXmlCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression, mmxe)
57+
)
5458
select sink.getNode(), source, sink,
5559
"MyBatis Mapper XML SQL injection might include code from $@ to $@.", source.getNode(),
5660
"this user input", mmxe, "this SQL operation"

0 commit comments

Comments
 (0)