rename isMybatisCollectionTypeSqlInjection

Author: haby0

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

@@ -53,7 +53,7 @@ where
   unsafeExpression = getAMybatisAnnotationSqlValue(isoa) and
   (
     isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, unsafeExpression) or
-    isMybatisAnnotationCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression)
+    isMybatisCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression)
   )
 select sink.getNode(), source, sink,
   "MyBatis annotation SQL injection might include code from $@ to $@.", source.getNode(),

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

@@ -76,7 +76,7 @@ string getAMybatisAnnotationSqlValue(IbatisSqlOperationAnnotation isoa) {
 
 /** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
 bindingset[unsafeExpression]
-predicate isMybatisAnnotationCollectionTypeSqlInjection(
+predicate isMybatisCollectionTypeSqlInjection(
   DataFlow::Node node, MethodAccess ma, string unsafeExpression
 ) {
   not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
@@ -100,38 +100,6 @@ predicate isMybatisAnnotationCollectionTypeSqlInjection(
   )
 }
 
-/** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
-bindingset[unsafeExpression]
-predicate isMybatisXmlCollectionTypeSqlInjection(
-  DataFlow::Node node, MethodAccess ma, string unsafeExpression, MyBatisMapperXMLElement mmxe
-) {
-  not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
-  // 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
-  //    Test test(Map map);
-  //    <select id="test" resultMap="BaseResultMap">
-  //        select id,name from test where name in
-  //            <foreach collection="list" item="value" open="(" close=")" separator=",">
-  //              ${value}
-  //            </foreach>
-  //    </select>
-  // ```
-  exists(int i, MyBatisMapperForeach mbmf |
-    mbmf = mmxe 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
-    unsafeExpression.matches("${%}") and
-    ma.getArgument(i) = node.asExpr()
-  )
-}
-
 /** Holds if `node` is an argument to `ma` that is vulnerable to SQL injection attacks if `unsafeExpression` occurs in a MyBatis SQL expression. */
 bindingset[unsafeExpression]
 predicate isMybatisXmlOrAnnotationSqlInjection(

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

@@ -52,8 +52,10 @@ where
   myBatisMapperXMLElementFromMethod(ma.getMethod(), mmxe) and
   unsafeExpression = getAMybatisXmlSetValue(mmxe) and
   (
-    isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, unsafeExpression) or
-    isMybatisXmlCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression, mmxe)
+    isMybatisXmlOrAnnotationSqlInjection(sink.getNode(), ma, unsafeExpression)
+    or
+    mmxe instanceof MyBatisMapperForeach and
+    isMybatisCollectionTypeSqlInjection(sink.getNode(), ma, unsafeExpression)
   )
 select sink.getNode(), source, sink,
   "MyBatis Mapper XML SQL injection might include code from $@ to $@.", source.getNode(),