java: add Escaping query (P1)

Author: yoff

java/ql/src/Likely Bugs/Concurrency/Escaping.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>
+In a thread-safe class, non-final fields should generally be private (or possibly volatile) to ensure that they cannot be accessed by other threads in an unsafe manner.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+If the field does not change, mark it as <code>final</code>. If the field is mutable, mark it as <code>private</code> and provide properly synchronized accessors.</p>
+
+</recommendation>
+
+<references>
+
+
+<li>
+  Java Language Specification, chapter 17:
+  <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
+</li>
+<li>
+  Java concurrency package:
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
+</li>
+
+
+</references>
+</qhelp>

java/ql/src/Likely Bugs/Concurrency/Escaping.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Escaping
+ * @kind problem
+ * @problem.severity warning
+ * @id java/escaping
+ */
+
+import java
+import semmle.code.java.ConflictingAccess
+
+from Field f, ClassAnnotatedAsThreadSafe c
+where
+  f = c.getAField() and
+  not f.isFinal() and // final fields do not change
+  not f.isPrivate() and
+  // We believe that protected fields are also dangerous
+  // Volatile fields cannot cause data races, but it is dubious to allow changes.
+  // For now, we ignore volatile fields, but there are likely bugs to be caught here.
+  not f.isVolatile()
+select f, "The class $@ is marked as thread-safe, but this field is potentially escaping.", c,
+  c.getName()

java/ql/test/query-tests/Escaping/Escaping.expected

@@ -0,0 +1,3 @@
+| Escaping.java:3:7:3:7 | x | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
+| Escaping.java:4:14:4:14 | y | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
+| Escaping.java:9:18:9:18 | b | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |

java/ql/test/query-tests/Escaping/Escaping.java

@@ -0,0 +1,17 @@
+@ThreadSafe
+public class Escaping {
+  int x;  //$ Alert
+  public int y = 0;  //$ Alert
+  private int z = 3;
+  final int w = 0;
+  public final int u = 4;
+  private final long a = 5;
+  protected long b = 0;  //$ Alert
+  protected final long c = 0L;
+  volatile long d = 3;
+  protected volatile long e = 3L;
+
+  public void methodLocal() {
+    int i;
+  }
+}

java/ql/test/query-tests/Escaping/Escaping.qlref

@@ -0,0 +1,2 @@
+query: Likely Bugs/Concurrency/Escaping.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/Escaping/ThreadSafe.java

@@ -0,0 +1,2 @@
+public @interface ThreadSafe {
+}