Merge pull request #11368 from ka1n4t/main

Java: Add binding between annotation and sink-param in MyBatis SQL Injection query

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

@@ -134,7 +134,9 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
           .matches("${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
               "%}") and
       annotation.getType() instanceof TypeParam and
-      ma.getAnArgument() = node.asExpr()
+      ma.getAnArgument() = node.asExpr() and
+      annotation.getTarget() =
+        ma.getMethod().getParameter(node.asExpr().(Argument).getParameterPos())
     )
     or
     // MyBatis default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].

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

@@ -1,16 +1,30 @@
 edges
 | MybatisSqlInjection.java:62:19:62:43 | name : String | MybatisSqlInjection.java:63:35:63:38 | name : String |
 | MybatisSqlInjection.java:63:35:63:38 | name : String | MybatisSqlInjectionService.java:48:19:48:29 | name : String |
+| MybatisSqlInjection.java:94:20:94:44 | name : String | MybatisSqlInjection.java:95:36:95:39 | name : String |
+| MybatisSqlInjection.java:95:36:95:39 | name : String | MybatisSqlInjectionService.java:76:20:76:30 | name : String |
+| MybatisSqlInjection.java:99:20:99:43 | age : String | MybatisSqlInjection.java:100:36:100:38 | age : String |
+| MybatisSqlInjection.java:100:36:100:38 | age : String | MybatisSqlInjectionService.java:80:20:80:29 | age : String |
 | MybatisSqlInjectionService.java:48:19:48:29 | name : String | MybatisSqlInjectionService.java:50:23:50:26 | name : String |
 | MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String | MybatisSqlInjectionService.java:51:27:51:33 | hashMap |
 | MybatisSqlInjectionService.java:50:23:50:26 | name : String | MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String |
+| MybatisSqlInjectionService.java:76:20:76:30 | name : String | MybatisSqlInjectionService.java:77:28:77:31 | name |
+| MybatisSqlInjectionService.java:80:20:80:29 | age : String | MybatisSqlInjectionService.java:81:28:81:30 | age |
 nodes
 | MybatisSqlInjection.java:62:19:62:43 | name : String | semmle.label | name : String |
 | MybatisSqlInjection.java:63:35:63:38 | name : String | semmle.label | name : String |
+| MybatisSqlInjection.java:94:20:94:44 | name : String | semmle.label | name : String |
+| MybatisSqlInjection.java:95:36:95:39 | name : String | semmle.label | name : String |
+| MybatisSqlInjection.java:99:20:99:43 | age : String | semmle.label | age : String |
+| MybatisSqlInjection.java:100:36:100:38 | age : String | semmle.label | age : String |
 | MybatisSqlInjectionService.java:48:19:48:29 | name : String | semmle.label | name : String |
 | MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String | semmle.label | hashMap [post update] [<map.value>] : String |
 | MybatisSqlInjectionService.java:50:23:50:26 | name : String | semmle.label | name : String |
 | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | semmle.label | hashMap |
+| MybatisSqlInjectionService.java:76:20:76:30 | name : String | semmle.label | name : String |
+| MybatisSqlInjectionService.java:77:28:77:31 | name | semmle.label | name |
+| MybatisSqlInjectionService.java:80:20:80:29 | age : String | semmle.label | age : String |
+| MybatisSqlInjectionService.java:81:28:81:30 | age | semmle.label | age |
 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 |

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

@@ -89,4 +89,14 @@ public class MybatisSqlInjection {
 	public void badInsert(@RequestParam String name) {
 		mybatisSqlInjectionService.badInsert(name);
 	}
+
+	@GetMapping(value = "good2")
+	public void good2(@RequestParam String name, @RequestParam Integer age) {
+		mybatisSqlInjectionService.good2(name, age);
+	}
+
+	@GetMapping(value = "good3")
+	public void good3(@RequestParam String age) {
+		mybatisSqlInjectionService.good3(age);
+	}
 }

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

@@ -72,4 +72,12 @@ public class MybatisSqlInjectionService {
 	public void badInsert(String input) {
 		sqlInjectionMapper.badInsert(input);
 	}
+
+	public void good2(String name, Integer age){
+		sqlInjectionMapper.good2(name, age);
+	}
+
+	public void good3(String age){
+		sqlInjectionMapper.good3(age);
+	}
 }

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

@@ -37,26 +37,33 @@ public interface SqlInjectionMapper {
 
 	//using providers
 	@SelectProvider(
-		type = MyBatisProvider.class,
-		method = "badSelect"
+			type = MyBatisProvider.class,
+			method = "badSelect"
 	)
 	String badSelect(String input);
 
 	@DeleteProvider(
-		type = MyBatisProvider.class,
-		method = "badDelete"
+			type = MyBatisProvider.class,
+			method = "badDelete"
 	)
 	void badDelete(String input);
 
 	@UpdateProvider(
-		type = MyBatisProvider.class,
-		method = "badUpdate"
+			type = MyBatisProvider.class,
+			method = "badUpdate"
 	)
 	void badUpdate(String input);
 
 	@InsertProvider(
-		type = MyBatisProvider.class,
-		method = "badInsert"
+			type = MyBatisProvider.class,
+			method = "badInsert"
 	)
 	void badInsert(String input);
+
+	@Select("select * from user_info where name = #{name} and age = ${age}")
+	String good2(@Param("name") String name, Integer age);
+
+	@Select("select * from user_info where age = #{age}")
+	String good3(@Param("age") String age);
+
 }