Java: handle MyBatis annotations for insert/update/delete

Author: Jami Cogswell

java/ql/lib/semmle/code/java/security/CsrfUnprotectedRequestTypeQuery.qll

@@ -36,9 +36,9 @@ private class SpringCsrfUnprotectedMethod extends CsrfUnprotectedMethod instance
 /** A method that updates a database. */
 abstract class DatabaseUpdateMethod extends Method { }
 
-/** A MyBatis Mapper method that updates a database. */
-private class MyBatisMapperDatabaseUpdateMethod extends DatabaseUpdateMethod {
-  MyBatisMapperDatabaseUpdateMethod() {
+/** A MyBatis method that updates a database. */
+private class MyBatisDatabaseUpdateMethod extends DatabaseUpdateMethod {
+  MyBatisDatabaseUpdateMethod() {
     exists(MyBatisMapperSqlOperation mapperXml |
       (
         mapperXml instanceof MyBatisMapperInsert or
@@ -47,6 +47,14 @@ private class MyBatisMapperDatabaseUpdateMethod extends DatabaseUpdateMethod {
       ) and
       this = mapperXml.getMapperMethod()
     )
+    or
+    exists(MyBatisSqlOperationAnnotationMethod m | this = m |
+      not m.getAnAnnotation().getType().hasQualifiedName("org.apache.ibatis.annotations", "Select")
+    )
+    or
+    exists(Method m | this = m |
+      m.hasAnnotation("org.apache.ibatis.annotations", ["Delete", "Update", "Insert"] + "Provider")
+    )
   }
 }
 

java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java

@@ -138,21 +138,39 @@ public void bad7(@RequestParam String name) { // $ hasCsrfUnprotectedRequestType
 		myBatisService.bad7(name);
 	}
 
-    // BAD: uses GET request when updating a database with `@DeleteProvider`
+    // BAD: uses GET request when updating a database with MyBatis `@DeleteProvider`
     @GetMapping(value = "badDelete")
 	public void badDelete(@RequestParam String name) { // $ hasCsrfUnprotectedRequestType
 		myBatisService.badDelete(name);
 	}
 
-    // BAD: uses GET request when updating a database with `@UpdateProvider`
+    // BAD: uses GET request when updating a database with MyBatis `@UpdateProvider`
 	@GetMapping(value = "badUpdate")
 	public void badUpdate(@RequestParam String name) { // $ hasCsrfUnprotectedRequestType
 		myBatisService.badUpdate(name);
 	}
 
-    // BAD: uses GET request when updating a database with `@InsertProvider`
+    // BAD: uses GET request when updating a database with MyBatis `@InsertProvider`
 	@GetMapping(value = "badInsert")
 	public void badInsert(@RequestParam String name) { // $ hasCsrfUnprotectedRequestType
 		myBatisService.badInsert(name);
 	}
+
+    // BAD: uses GET request when updating a database with MyBatis `@Delete`
+    @GetMapping(value = "bad8")
+	public void bad8(@RequestParam int id) { // $ hasCsrfUnprotectedRequestType
+		myBatisService.bad8(id);
+	}
+
+    // BAD: uses GET request when updating a database with MyBatis `@Insert`
+    @GetMapping(value = "bad9")
+	public void bad9(@RequestParam String user) { // $ hasCsrfUnprotectedRequestType
+		myBatisService.bad9(user);
+	}
+
+    // BAD: uses GET request when updating a database with MyBatis `@Update`
+    @GetMapping(value = "bad10")
+	public void bad10(@RequestParam String user) { // $ hasCsrfUnprotectedRequestType
+		myBatisService.bad10(user);
+	}
 }

java/ql/test/query-tests/security/CWE-352/MyBatisMapper.java

@@ -3,6 +3,9 @@
 import org.apache.ibatis.annotations.DeleteProvider;
 import org.apache.ibatis.annotations.UpdateProvider;
 import org.apache.ibatis.annotations.InsertProvider;
+import org.apache.ibatis.annotations.Delete;
+import org.apache.ibatis.annotations.Update;
+import org.apache.ibatis.annotations.Insert;
 
 @Mapper
 @Repository
@@ -28,4 +31,13 @@ public interface MyBatisMapper {
 			method = "badInsert"
 	)
 	void badInsert(String input);
+
+	@Delete("DELETE FROM users WHERE id = #{id}")
+   	boolean bad8(int id);
+
+	@Insert("INSERT INTO users (id, name) VALUES(#{id}, #{name})")
+   	void bad9(String user);
+
+	@Update("UPDATE users SET name = #{name} WHERE id = #{id}")
+   	boolean bad10(String user);
 }

java/ql/test/query-tests/security/CWE-352/MyBatisService.java

@@ -22,4 +22,16 @@ public void badUpdate(String input) {
 	public void badInsert(String input) {
 		myBatisMapper.badInsert(input);
 	}
+
+	public void bad8(int id){
+		myBatisMapper.bad8(id);
+	}
+
+	public void bad9(String user){
+		myBatisMapper.bad9(user);
+	}
+
+	public void bad10(String user){
+		myBatisMapper.bad10(user);
+	}
 }