Merge pull request #358 from aschackmull/java/sql-sinks

Approved by yh-semmle

Author: semmle-qlci

change-notes/1.19/analysis-java.md

@@ -12,6 +12,8 @@
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Array index out of bounds (`java/index-out-of-bounds`) | Fewer false positive results | False positives involving arrays with a length evenly divisible by 3 or some greater number and an index being increased with a similar stride length are no longer reported. |
+| Query built from user-controlled sources (`java/sql-injection`) | More results | Sql injection sinks from the Spring JDBC, MyBatis, and Hibernate frameworks are now reported. |
+| Query built without neutralizing special characters (`java/concatenated-sql-query`) | More results | Sql injection sinks from the Spring JDBC, MyBatis, and Hibernate frameworks are now reported. |
 | Unreachable catch clause (`java/unreachable-catch-clause`) | Fewer false positive results | This rule now accounts for calls to generic methods that throw generic exceptions. |
 | Useless comparison test (`java/constant-comparison`) | Fewer false positive results | Constant comparisons guarding `java.util.ConcurrentModificationException` are no longer reported, as they are intended to always be false in the absence of API misuse. |
 

java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll

@@ -4,6 +4,9 @@ import semmle.code.java.Expr
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.frameworks.android.SQLite
 import semmle.code.java.frameworks.javaee.Persistence
+import semmle.code.java.frameworks.SpringJdbc
+import semmle.code.java.frameworks.MyBatis
+import semmle.code.java.frameworks.Hibernate
 
 /** A sink for database query language injection vulnerabilities. */
 abstract class QueryInjectionSink extends DataFlow::ExprNode { }
@@ -13,8 +16,19 @@ class SqlInjectionSink extends QueryInjectionSink {
   SqlInjectionSink() {
     this.getExpr() instanceof SqlExpr
     or
-    exists(SQLiteRunner s, MethodAccess m | m.getMethod() = s |
-      m.getArgument(s.sqlIndex()) = this.getExpr()
+    exists(MethodAccess ma, Method m, int index |
+      ma.getMethod() = m and
+      ma.getArgument(index) = this.getExpr()
+    |
+      index = m.(SQLiteRunner).sqlIndex()
+      or
+      m instanceof BatchUpdateVarargsMethod
+      or
+      index = 0 and jdbcSqlMethod(m)
+      or
+      index = 0 and mybatisSqlMethod(m)
+      or
+      index = 0 and hibernateSqlMethod(m)
     )
   }
 }

java/ql/src/semmle/code/java/frameworks/Hibernate.qll

@@ -0,0 +1,23 @@
+/**
+ * Provides classes and predicates for working with the Hibernate framework.
+ */
+
+import java
+
+/** The interface `org.hibernate.Session`. */
+class HibernateSession extends RefType {
+  HibernateSession() { this.hasQualifiedName("org.hibernate", "Session") }
+}
+
+/**
+ * Holds if `m` is a method on `HibernateSession` taking an SQL string as its
+ * first argument.
+ */
+predicate hibernateSqlMethod(Method m) {
+  m.getDeclaringType() instanceof HibernateSession and
+  m.getParameterType(0) instanceof TypeString and
+  (
+    m.hasName("createQuery") or
+    m.hasName("createSQLQuery")
+  )
+}

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

@@ -0,0 +1,27 @@
+/**
+ * Provides classes and predicates for working with the MyBatis framework.
+ */
+
+import java
+
+/** The class `org.apache.ibatis.jdbc.SqlRunner`. */
+class MyBatisSqlRunner extends RefType {
+  MyBatisSqlRunner() { this.hasQualifiedName("org.apache.ibatis.jdbc", "SqlRunner") }
+}
+
+/**
+ * Holds if `m` is a method on `MyBatisSqlRunner` taking an SQL string as its
+ * first argument.
+ */
+predicate mybatisSqlMethod(Method m) {
+  m.getDeclaringType() instanceof MyBatisSqlRunner and
+  m.getParameterType(0) instanceof TypeString and
+  (
+    m.hasName("delete") or
+    m.hasName("insert") or
+    m.hasName("run") or
+    m.hasName("selectAll") or
+    m.hasName("selectOne") or
+    m.hasName("update")
+  )
+}

java/ql/src/semmle/code/java/frameworks/SpringJdbc.qll

@@ -0,0 +1,35 @@
+/**
+ * Provides classes and predicates for working with the Spring JDBC framework.
+ */
+
+import java
+
+/** The class `org.springframework.jdbc.core.JdbcTemplate`. */
+class JdbcTemplate extends RefType {
+  JdbcTemplate() { this.hasQualifiedName("org.springframework.jdbc.core", "JdbcTemplate") }
+}
+
+/**
+ * Holds if `m` is a method on `JdbcTemplate` taking an SQL string as its first
+ * argument.
+ */
+predicate jdbcSqlMethod(Method m) {
+  m.getDeclaringType() instanceof JdbcTemplate and
+  m.getParameterType(0) instanceof TypeString and
+  (
+    m.hasName("batchUpdate") or
+    m.hasName("execute") or
+    m.getName().matches("query%") or
+    m.hasName("update")
+  )
+}
+
+/** The method `JdbcTemplate.batchUpdate(String... sql)` */
+class BatchUpdateVarargsMethod extends Method {
+  BatchUpdateVarargsMethod() {
+    this.getDeclaringType() instanceof JdbcTemplate and
+    this.hasName("batchUpdate") and
+    this.getParameterType(0).(Array).getComponentType() instanceof TypeString and
+    this.getParameter(0).isVarargs()
+  }
+}