MyBatisAnnotationSqlInjection no @param case

Author: retanoj

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

@@ -185,5 +185,21 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
       unsafeExpression.matches("${%}") and
       ma.getAnArgument() = node.asExpr()
     )
+    or
+    // Some of method parameters are not annotated with `@Param`, which named in the SQL statement as their name.
+    // Improper use of these parameters has a SQL injection vulnerability.
+    // e.g.
+    //
+    // ```java
+    //    @Select(select id,name from test where id = #{id} or name = '${name}')
+    //    Test test(Integer id, String name);
+    // ```
+    exists(Parameter param, int idx |
+      param = ma.getMethod().getParameter(idx)
+    |
+      not param.getAnAnnotation().getType() instanceof TypeParam and
+      unsafeExpression.matches("${" + param.getName() + "}") and
+      ma.getArgument(idx) = node.asExpr()
+    )
   )
 }

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

@@ -63,6 +63,11 @@ public void bad9(@RequestParam String name) {
 		mybatisSqlInjectionService.bad9(name);
 	}
 
+	@GetMapping(value = "msi10")
+	public void bad10(@RequestParam Integer id, @RequestParam String name) {
+		mybatisSqlInjectionService.bad10(id, name);
+	}
+
 	@GetMapping(value = "good1")
 	public List<Test> good1(Integer id) {
 		List<Test> result = mybatisSqlInjectionService.good1(id);
@@ -99,4 +104,9 @@ public void good2(@RequestParam String name, @RequestParam Integer age) {
 	public void good3(@RequestParam String age) {
 		mybatisSqlInjectionService.good3(age);
 	}
+
+	@GetMapping(value = "good4")
+	public void bad10(@RequestParam Integer id, @RequestParam String name) {
+		mybatisSqlInjectionService.good4(id, name);
+	}
 }

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

@@ -51,6 +51,10 @@ public void bad9(String name) {
 		sqlInjectionMapper.bad9(hashMap);
 	}
 
+	public void bad9(Integer id, String name) {
+		sqlInjectionMapper.bad10(id, name);
+	}
+
 	public List<Test> good1(Integer id) {
 		List<Test> result = sqlInjectionMapper.good1(id);
 		return result;
@@ -80,4 +84,8 @@ public void good2(String name, Integer age){
 	public void good3(String age){
 		sqlInjectionMapper.good3(age);
 	}
+
+	public void good4(Integer id, String name) {
+		sqlInjectionMapper.good4(id, name);
+	}
 }

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

@@ -33,6 +33,9 @@ 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}'"})
+	String bad10(Integer id, String name);
+
 	List<Test> good1(Integer id);
 
 	//using providers
@@ -66,4 +69,6 @@ public interface SqlInjectionMapper {
 	@Select("select * from user_info where age = #{age}")
 	String good3(@Param("age") String age);
 
+	@Select({"select * from test where id = #{id} and name = #{name}"})
+	String good4(Integer id, String name);
 }