consider blankspace / comma /dot field

Author: retanoj

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

@@ -86,7 +86,7 @@ bindingset[unsafeExpression]
 predicate isMybatisCollectionTypeSqlInjection(
   DataFlow::Node node, MethodAccess ma, string unsafeExpression
 ) {
-  not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
+  not unsafeExpression.regexpMatch("\\$\\{\\s*" + getAMybatisConfigurationVariableKey() + "\\s*\\}") 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.
@@ -120,19 +120,22 @@ bindingset[unsafeExpression]
 predicate isMybatisXmlOrAnnotationSqlInjection(
   DataFlow::Node node, MethodAccess ma, string unsafeExpression
 ) {
-  not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
+  not unsafeExpression.regexpMatch("\\$\\{\\s*" + getAMybatisConfigurationVariableKey() + "\\s*\\}") 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);
+    //    
+    //    @Select(select id,name from test where name = ${ user . name })
+    //    void test(@Param("user") User u);
     // ```
     exists(Annotation annotation |
       unsafeExpression
-          .matches("${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
-              "%}") and
+          .regexpMatch("\\$\\{\\s*" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
+              "\\b[^}]*?\\}") and
       annotation.getType() instanceof TypeParam and
       ma.getAnArgument() = node.asExpr() and
       annotation.getTarget() =
@@ -150,11 +153,11 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
     exists(int i |
       not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
       (
-        unsafeExpression.matches("${param" + (i + 1) + "%}")
+        unsafeExpression.regexpMatch("\\$\\{\\s*param" + (i + 1) + "\\b[^}]*?\\}")
         or
-        unsafeExpression.matches("${arg" + i + "%}")
+        unsafeExpression.regexpMatch("\\$\\{\\s*arg" + i + "\\b[^}]*?\\}")
         or
-        unsafeExpression.regexpMatch("\\$\\{\\s*" + ma.getMethod().getParameter(i).getName() + "\\s*(,.*?)?\\s*\\}")
+        unsafeExpression.regexpMatch("\\$\\{\\s*" + ma.getMethod().getParameter(i).getName() + "\\b[^}]*?\\}")
       ) and
       ma.getArgument(i) = node.asExpr()
     )
@@ -169,7 +172,7 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
     exists(int i, RefType t |
       not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
       ma.getMethod().getParameterType(i).getName() = t.getName() and
-      unsafeExpression.matches("${" + t.getAField().getName() + "%}") and
+      unsafeExpression.regexpMatch("\\$\\{\\s*" + t.getAField().getName() + "\\b[^}]*?\\}") and
       ma.getArgument(i) = node.asExpr()
     )
     or

java/ql/test/experimental/query-tests/security/CWE-089/src/main/MyBatisAnnotationSqlInjection.expected

@@ -42,4 +42,4 @@ nodes
 subpaths
 #select
 | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MybatisSqlInjection.java:62:19:62:43 | name : String | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:62:19:62:43 | name | this user input | SqlInjectionMapper.java:33:2:33:54 | Select | this SQL operation |
-| MybatisSqlInjectionService.java:55:32:55:35 | name | MybatisSqlInjection.java:67:46:67:70 | name : String | MybatisSqlInjectionService.java:55:32:55:35 | name | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:67:46:67:70 | name | this user input | SqlInjectionMapper.java:36:2:36:70 | Select | this SQL operation |
+| MybatisSqlInjectionService.java:55:32:55:35 | name | MybatisSqlInjection.java:67:46:67:70 | name : String | MybatisSqlInjectionService.java:55:32:55:35 | name | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:67:46:67:70 | name | this user input | SqlInjectionMapper.java:36:2:36:72 | Select | this SQL operation |

java/ql/test/experimental/query-tests/security/CWE-089/src/main/SqlInjectionMapper.java

@@ -33,7 +33,7 @@ public interface SqlInjectionMapper {
 	@Select({"select * from test", "where id = ${name}"})
 	public Test bad9(HashMap<String, Object> map);
 
-	@Select({"select * from test where id = #{id} and name = '${name}'"})
+	@Select({"select * from test where id = #{id} and name = '${ name }'"})
 	String bad10(Integer id, String name);
 
 	List<Test> good1(Integer id);

java/ql/test/experimental/query-tests/security/CWE-089/src/main/SqlInjectionMapper.xml

@@ -12,7 +12,7 @@
   <sql id="Update_By_Example_Where_Clause">
     <where>
       <if test="test.name != null">
-        and name = ${test.name,jdbcType=VARCHAR}
+        and name = ${ test . name , jdbcType = VARCHAR }
       </if>
       <if test="test.id != null">
         and id = #{test.id}