Java: Add MyBatis' Providers sinks

java/ql/lib/change-notes/2022-03-15-mybatis-providers.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added modeling of MyBatis (`org.apache.ibatis`) Providers, resulting in additional sinks for the queries `java/ognl-injection`, `java/sql-injection`, `java/sql-injection-local` and `java/concatenated-sql-query`.

java/ql/lib/semmle/code/java/frameworks/MyBatis.qll

@@ -3,6 +3,8 @@
  */
 
 import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.dataflow.ExternalFlow
 
 /** The class `org.apache.ibatis.jdbc.SqlRunner`. */
@@ -102,3 +104,116 @@ class MyBatisSqlOperationAnnotationMethod extends Method {
 class TypeParam extends Interface {
   TypeParam() { this.hasQualifiedName("org.apache.ibatis.annotations", "Param") }
 }
+
+private class MyBatisProvider extends RefType {
+  MyBatisProvider() {
+    this.hasQualifiedName("org.apache.ibatis.annotations",
+      ["Select", "Delete", "Insert", "Update"] + "Provider")
+  }
+}
+
+/**
+ * A return statement of a method used in a MyBatis Provider.
+ *
+ * See
+ * - `MyBatisProvider`
+ * - https://mybatis.org/mybatis-3/apidocs/org/apache/ibatis/annotations/package-summary.html
+ */
+class MyBatisInjectionSink extends DataFlow::Node {
+  MyBatisInjectionSink() {
+    exists(Annotation a, Method m |
+      a.getType() instanceof MyBatisProvider and
+      m.getDeclaringType() = a.getValue(["type", "value"]).(TypeLiteral).getTypeName().getType() and
+      m.hasName(a.getValue("method").(StringLiteral).getValue()) and
+      exists(ReturnStmt ret | this.asExpr() = ret.getResult() and ret.getEnclosingCallable() = m)
+    )
+  }
+}
+
+private class MyBatisProviderStep extends TaintTracking::AdditionalValueStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(MethodAccess ma, Annotation a, Method providerMethod |
+      exists(int i |
+        ma.getArgument(pragma[only_bind_into](i)) = n1.asExpr() and
+        providerMethod.getParameter(pragma[only_bind_into](i)) = n2.asParameter()
+      )
+    |
+      a.getType() instanceof MyBatisProvider and
+      ma.getMethod().getAnAnnotation() = a and
+      providerMethod.getDeclaringType() =
+        a.getValue(["type", "value"]).(TypeLiteral).getTypeName().getType() and
+      providerMethod.hasName(a.getValue("method").(StringLiteral).getValue())
+    )
+  }
+}
+
+private class MyBatisAbstractSqlToStringStep extends SummaryModelCsv {
+  override predicate row(string row) {
+    row = "org.apache.ibatis.jdbc;AbstractSQL;true;toString;;;Argument[-1];ReturnValue;taint"
+  }
+}
+
+private class MyBatisAbstractSqlMethodsStep extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.apache.ibatis.jdbc;AbstractSQL;true;toString;;;Argument[-1];ReturnValue;taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;VALUES;(String,String);;Argument[0..1];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;UPDATE;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;SELECT;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OFFSET_ROWS;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;OFFSET;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;LIMIT;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INTO_VALUES;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INTO_COLUMNS;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INSERT_INTO;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String[]);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String);;Argument[0].ArrayElement;Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;FETCH_FIRST_ROWS_ONLY;(String);;Argument[0];Argument[-1];taint",
+        "org.apache.ibatis.jdbc;AbstractSQL;true;DELETE_FROM;(String);;Argument[0];Argument[-1];taint"
+      ]
+  }
+}

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

@@ -3,6 +3,7 @@
 import java
 private import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.frameworks.MyBatis
 
 /**
  * A data flow sink for unvalidated user input that is used in OGNL EL evaluation.
@@ -122,3 +123,5 @@ private class DefaultOgnlInjectionAdditionalTaintStep extends OgnlInjectionAddit
     setExpressionStep(node1, node2)
   }
 }
+
+private class MyBatisOgnlInjectionSink extends OgnlInjectionSink instanceof MyBatisInjectionSink { }

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

@@ -3,6 +3,7 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.frameworks.javaee.Persistence
+private import semmle.code.java.frameworks.MyBatis
 import semmle.code.java.dataflow.ExternalFlow
 
 /** A sink for database query language injection vulnerabilities. */
@@ -66,3 +67,5 @@ private class MongoJsonStep extends AdditionalQueryInjectionTaintStep {
     )
   }
 }
+
+private class MyBatisSqlInjectionSink extends QueryInjectionSink instanceof MyBatisInjectionSink { }

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

@@ -13,4 +13,4 @@ nodes
 | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | semmle.label | hashMap |
 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:29:2:29:54 | Select | this SQL operation |
+| 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/MyBatisProvider.java

@@ -0,0 +1,35 @@
+import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.jdbc.SQL;
+
+public class MyBatisProvider {
+    public String badSelect(@Param("input") final String input) {
+        String s = (new SQL() {
+            {
+                this.SELECT("password");
+                this.FROM("users");
+                this.WHERE("username = '" + input + "'");
+            }
+        }).toString();
+        return s;
+    }
+
+    public String badDelete(@Param("input") final String input) {
+        return "DELETE FROM users WHERE username = '" + input + "';";
+    }
+
+    public String badUpdate(@Param("input") final String input) {
+        String s = (new SQL() {
+            {
+                this.UPDATE("users");
+                this.SET("balance = 0");
+                this.WHERE("username = '" + input + "'");
+            }
+        }).toString();
+        return s;
+    }
+
+    public String badInsert(@Param("input") final String input) {
+        return "INSERT INTO users VALUES (1, '" + input + "', 'hunter2');";
+    }
+}
+

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

@@ -68,4 +68,25 @@ public List<Test> good1(Integer id) {
 		List<Test> result = mybatisSqlInjectionService.good1(id);
 		return result;
 	}
+
+	// using providers
+	@GetMapping(value = "badSelect")
+	public String badSelect(@RequestParam String name) {
+		return mybatisSqlInjectionService.badSelect(name);
+	}
+
+	@GetMapping(value = "badDelete")
+	public void badDelete(@RequestParam String name) {
+		mybatisSqlInjectionService.badDelete(name);
+	}
+
+	@GetMapping(value = "badUpdate")
+	public void badUpdate(@RequestParam String name) {
+		mybatisSqlInjectionService.badUpdate(name);
+	}
+
+	@GetMapping(value = "badInsert")
+	public void badInsert(@RequestParam String name) {
+		mybatisSqlInjectionService.badInsert(name);
+	}
 }

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

@@ -55,4 +55,21 @@ public List<Test> good1(Integer id) {
 		List<Test> result = sqlInjectionMapper.good1(id);
 		return result;
 	}
+
+	// using providers
+	public String badSelect(String input) {
+		return sqlInjectionMapper.badSelect(input);
+	}
+
+	public void badDelete(String input) {
+		sqlInjectionMapper.badDelete(input);
+	}
+
+	public void badUpdate(String input) {
+		sqlInjectionMapper.badUpdate(input);
+	}
+
+	public void badInsert(String input) {
+		sqlInjectionMapper.badInsert(input);
+	}
 }

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

@@ -5,6 +5,10 @@
 import org.apache.ibatis.annotations.Param;
 import org.springframework.stereotype.Repository;
 import org.apache.ibatis.annotations.Select;
+import org.apache.ibatis.annotations.SelectProvider;
+import org.apache.ibatis.annotations.DeleteProvider;
+import org.apache.ibatis.annotations.UpdateProvider;
+import org.apache.ibatis.annotations.InsertProvider;
 
 @Mapper
 @Repository
@@ -30,4 +34,29 @@ public interface SqlInjectionMapper {
 	public Test bad9(HashMap<String, Object> map);
 
 	List<Test> good1(Integer id);
+
+	//using providers
+	@SelectProvider(
+		type = MyBatisProvider.class,
+		method = "badSelect"
+	)
+	String badSelect(String input);
+
+	@DeleteProvider(
+		type = MyBatisProvider.class,
+		method = "badDelete"
+	)
+	void badDelete(String input);
+
+	@UpdateProvider(
+		type = MyBatisProvider.class,
+		method = "badUpdate"
+	)
+	void badUpdate(String input);
+
+	@InsertProvider(
+		type = MyBatisProvider.class,
+		method = "badInsert"
+	)
+	void badInsert(String input);
 }