Fix query java/internal-representation-exposure regarding generic callees, and add a test

Author: smowton

java/ql/src/Violations of Best Practice/Implementation Hiding/ExposeRepresentation.ql

@@ -72,7 +72,7 @@ predicate mayWriteToArray(Expr modified) {
   // return __array__;    ...  method()[1] = 0
   exists(ReturnStmt rs | modified = rs.getResult() and relevantType(modified.getType()) |
     exists(Callable enclosing, MethodAccess ma |
-      enclosing = rs.getEnclosingCallable() and ma.getMethod() = enclosing
+      enclosing = rs.getEnclosingCallable() and ma.getMethod().getSourceDeclaration() = enclosing
     |
       mayWriteToArray(ma)
     )
@@ -100,7 +100,7 @@ VarAccess varPassedInto(Callable c, int i) {
 predicate exposesByReturn(Callable c, Field f, Expr why, string whyText) {
   returnsArray(c, f) and
   exists(MethodAccess ma |
-    ma.getMethod() = c and ma.getCompilationUnit() != c.getCompilationUnit()
+    ma.getMethod().getSourceDeclaration() = c and ma.getCompilationUnit() != c.getCompilationUnit()
   |
     mayWriteToArray(ma) and
     why = ma and

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

@@ -0,0 +1,7 @@
+| ExposesRep.java:11:19:11:28 | getStrings | getStrings exposes the internal representation stored in field strings. The value may be modified $@. | User.java:5:5:5:19 | User.java:5:5:5:19 | after this call to getStrings |
+| ExposesRep.java:11:19:11:28 | getStrings | getStrings exposes the internal representation stored in field strings. The value may be modified $@. | User.java:13:12:13:26 | User.java:13:12:13:26 | after this call to getStrings |
+| ExposesRep.java:11:19:11:28 | getStrings | getStrings exposes the internal representation stored in field strings. The value may be modified $@. | User.java:38:12:38:26 | User.java:38:12:38:26 | after this call to getStrings |
+| ExposesRep.java:13:30:13:41 | getStringMap | getStringMap exposes the internal representation stored in field stringMap. The value may be modified $@. | User.java:9:5:9:21 | User.java:9:5:9:21 | after this call to getStringMap |
+| ExposesRep.java:17:15:17:24 | setStrings | setStrings exposes the internal representation stored in field strings. The value may be modified $@. | User.java:22:5:22:6 | User.java:22:5:22:6 | through the variable ss |
+| ExposesRep.java:21:15:21:26 | setStringMap | setStringMap exposes the internal representation stored in field stringMap. The value may be modified $@. | User.java:27:5:27:5 | User.java:27:5:27:5 | through the variable m |
+| ExposesRep.java:29:14:29:21 | getArray | getArray exposes the internal representation stored in field array. The value may be modified $@. | User.java:31:5:31:18 | User.java:31:5:31:18 | after this call to getArray |

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

@@ -0,0 +1 @@
+Violations of Best Practice/Implementation Hiding/ExposeRepresentation.ql

java/ql/test/query-tests/ExposeRepresentation/ExposesRep.java

@@ -0,0 +1,30 @@
+import java.util.Map;
+
+public class ExposesRep {
+  private String[] strings;
+  private Map<String, String> stringMap;
+
+  public ExposesRep() {
+    strings = new String[1];
+  }
+
+  public String[] getStrings() { return strings; }
+
+  public Map<String, String> getStringMap() {
+    return stringMap;
+  }
+
+  public void setStrings(String[] ss) {
+    this.strings = ss;
+  }
+
+  public void setStringMap(Map<String, String> m) {
+    this.stringMap = m;
+  }
+}
+
+class GenericExposesRep<T> {
+  private T[] array;
+
+  public T[] getArray() { return array; }
+}

java/ql/test/query-tests/ExposeRepresentation/User.java

@@ -0,0 +1,45 @@
+import java.util.Map;
+
+public class User {
+  public static void test1(ExposesRep er) {
+    er.getStrings()[0] = "Hello world";
+  }
+
+  public static void test2(ExposesRep er) {
+    er.getStringMap().put("Hello", "world");
+  }
+
+  public String[] indirectGetStrings(ExposesRep er) {
+    return er.getStrings();
+  }
+
+  public void test3(ExposesRep er) {
+    indirectGetStrings(er)[0] = "Hello world";
+  }
+
+  public static void test4(ExposesRep er, String[] ss) {
+    er.setStrings(ss);
+    ss[0] = "Hello world";
+  }
+
+  public static void test5(ExposesRep er, Map<String, String> m) {
+    er.setStringMap(m);
+    m.put("Hello", "world");
+  }
+
+  public static void test6(GenericExposesRep<String> ger) {
+    ger.getArray()[0] = "Hello world";
+  }
+}
+
+class GenericUser<T> {
+
+  public String[] indirectGetStrings(ExposesRep er) {
+    return er.getStrings();
+  }
+
+  public static void test1(ExposesRep er, GenericUser<String> gu) {
+    gu.indirectGetStrings(er)[0] = "Hello world";
+  }
+
+}