Merge pull request github#3918 from aibaars/organise-container-flow

Java: Clean up ContainerFlow, consider more methods

Author: aschackmull

java/ql/src/semmle/code/java/dataflow/internal/ContainerFlow.qll

@@ -89,81 +89,189 @@ class ContainerType extends RefType {
 }
 
 private predicate taintPreservingQualifierToMethod(Method m) {
+  // java.util.Map.Entry
   m.getDeclaringType() instanceof EntryType and
-  m.hasName("getValue")
+  m.hasName(["getValue", "setValue"])
   or
+  // java.util.Iterable
   m.getDeclaringType() instanceof IterableType and
-  m.hasName("iterator")
+  m.hasName(["iterator", "spliterator"])
   or
+  // java.util.Iterator
   m.getDeclaringType() instanceof IteratorType and
   m.hasName("next")
   or
+  // java.util.ListIterator
+  m.getDeclaringType() instanceof IteratorType and
+  m.hasName("previous")
+  or
+  // java.util.Enumeration
   m.getDeclaringType() instanceof EnumerationType and
-  m.hasName("nextElement")
+  m.hasName(["asIterator", "nextElement"])
   or
-  m.(MapMethod).hasName("entrySet")
+  // java.util.Map
+  m
+      .(MapMethod)
+      .hasName(["computeIfAbsent", "entrySet", "get", "getOrDefault", "put", "putIfAbsent",
+            "remove", "replace", "values"])
   or
-  m.(MapMethod).hasName("get")
+  // java.util.Collection
+  m.(CollectionMethod).hasName(["parallelStream", "stream", "toArray"])
   or
-  m.(MapMethod).hasName("remove")
+  // java.util.List
+  m.(CollectionMethod).hasName(["get", "listIterator", "set", "subList"])
   or
-  m.(MapMethod).hasName("values")
+  m.(CollectionMethod).hasName("remove") and m.getParameterType(0).(PrimitiveType).hasName("int")
   or
-  m.(CollectionMethod).hasName("toArray")
+  // java.util.Vector
+  m.(CollectionMethod).hasName(["elementAt", "elements", "firstElement", "lastElement"])
   or
-  m.(CollectionMethod).hasName("get")
+  // java.util.Stack
+  m.(CollectionMethod).hasName(["peek", "pop"])
   or
-  m.(CollectionMethod).hasName("remove") and m.getParameterType(0).(PrimitiveType).hasName("int")
+  // java.util.Queue
+  m.(CollectionMethod).hasName(["element", "poll"])
   or
   m.(CollectionMethod).hasName("remove") and m.getNumberOfParameters() = 0
   or
-  m.(CollectionMethod).hasName("subList")
+  // java.util.Deque
+  m
+      .(CollectionMethod)
+      .hasName(["getFirst", "getLast", "peekFirst", "peekLast", "pollFirst", "pollLast",
+            "removeFirst", "removeLast"])
+  or
+  // java.util.concurrent.BlockingQueue
+  // covered by Queue: poll(long, TimeUnit)
+  m.(CollectionMethod).hasName("take")
+  or
+  // java.util.concurrent.BlockingDeque
+  // covered by Deque: pollFirst(long, TimeUnit), pollLast(long, TimeUnit)
+  m.(CollectionMethod).hasName(["takeFirst", "takeLast"])
+  or
+  // java.util.SortedSet
+  m.(CollectionMethod).hasName(["first", "headSet", "last", "subSet", "tailSet"])
   or
-  m.(CollectionMethod).hasName("firstElement")
+  // java.util.NavigableSet
+  // covered by Deque: pollFirst(), pollLast()
+  // covered by SortedSet: headSet(E, boolean), subSet(E, boolean, E, boolean) and tailSet(E, boolean)
+  m
+      .(CollectionMethod)
+      .hasName(["ceiling", "descendingIterator", "descendingSet", "floor", "higher", "lower"])
   or
-  m.(CollectionMethod).hasName("lastElement")
+  // java.util.SortedMap
+  m.(MapMethod).hasName(["headMap", "subMap", "tailMap"])
   or
-  m.(CollectionMethod).hasName("poll")
+  // java.util.NavigableMap
+  // covered by SortedMap: headMap(K, boolean), subMap(K, boolean, K, boolean), tailMap(K, boolean)
+  m
+      .(MapMethod)
+      .hasName(["ceilingEntry", "descendingMap", "firstEntry", "floorEntry", "higherEntry",
+            "lastEntry", "lowerEntry", "pollFirstEntry", "pollLastEntry"])
   or
-  m.(CollectionMethod).hasName("peek")
+  // java.util.Dictionary
+  m
+      .getDeclaringType()
+      .getSourceDeclaration()
+      .getASourceSupertype*()
+      .hasQualifiedName("java.util", "Dictionary") and
+  m.hasName(["elements", "get", "put", "remove"])
   or
-  m.(CollectionMethod).hasName("element")
+  // java.util.concurrent.ConcurrentHashMap
+  m.(MapMethod).hasName(["elements", "search", "searchEntries", "searchValues"])
 }
 
 private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
   taintPreservingQualifierToMethod(sink.getMethod()) and
   tracked = sink.getQualifier()
 }
 
-private predicate qualifierToArgumentStep(Expr tracked, RValue sink) {
-  exists(MethodAccess ma |
-    ma.getMethod().(CollectionMethod).hasName("toArray") and
+private predicate qualifierToArgumentStep(Expr tracked, Expr sink) {
+  exists(MethodAccess ma, CollectionMethod method |
+    method = ma.getMethod() and
+    (
+      // java.util.Vector
+      method.hasName("copyInto")
+      or
+      // java.util.concurrent.BlockingQueue
+      method.hasName("drainTo")
+      or
+      // java.util.Collection
+      method.hasName("toArray") and method.getParameter(0).getType() instanceof Array
+    ) and
     tracked = ma.getQualifier() and
-    sink = ma.getArgument(1)
+    sink = ma.getArgument(0)
   )
 }
 
 private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
-  method.(MapMethod).hasName("put") and arg = 1
+  // java.util.Map.Entry
+  method.getDeclaringType() instanceof EntryType and
+  method.hasName("setValue") and
+  arg = 0
   or
-  method.(MapMethod).hasName("putAll") and arg = 0
+  // java.util.Map
+  method.(MapMethod).hasName(["merge", "put", "putIfAbsent"]) and arg = 1
   or
-  method.(CollectionMethod).hasName("add") and arg = method.getNumberOfParameters() - 1
+  method.(MapMethod).hasName("replace") and arg = method.getNumberOfParameters() - 1
   or
-  method.(CollectionMethod).hasName("addAll") and arg = method.getNumberOfParameters() - 1
+  method.(MapMethod).hasName("putAll") and arg = 0
+  or
+  // java.util.ListIterator
+  method.getDeclaringType() instanceof IteratorType and
+  method.hasName(["add", "set"]) and
+  arg = 0
   or
-  method.(CollectionMethod).hasName("addElement") and arg = 0
+  // java.util.Collection
+  method.(CollectionMethod).hasName(["add", "addAll"]) and
+  // Refer to the last parameter to also cover List::add(int, E) and List::addAll(int, Collection)
+  arg = method.getNumberOfParameters() - 1
   or
+  // java.util.List
+  // covered by Collection: add(int, E), addAll(int, Collection<? extends E>)
   method.(CollectionMethod).hasName("set") and arg = 1
   or
+  // java.util.Vector
+  method.(CollectionMethod).hasName(["addElement", "insertElementAt", "setElementAt"]) and arg = 0
+  or
+  // java.util.Stack
+  method.(CollectionMethod).hasName("push") and arg = 0
+  or
+  // java.util.Queue
   method.(CollectionMethod).hasName("offer") and arg = 0
+  or
+  // java.util.Deque
+  // covered by Stack: push(E)
+  method.(CollectionMethod).hasName(["addFirst", "addLast", "offerFirst", "offerLast"]) and arg = 0
+  or
+  // java.util.concurrent.BlockingQueue
+  // covered by Queue: offer(E, long, TimeUnit)
+  method.(CollectionMethod).hasName("put") and arg = 0
+  or
+  // java.util.concurrent.TransferQueue
+  method.(CollectionMethod).hasName(["transfer", "tryTransfer"]) and arg = 0
+  or
+  // java.util.concurrent.BlockingDeque
+  // covered by Deque: offerFirst(E, long, TimeUnit), offerLast(E, long, TimeUnit)
+  method.(CollectionMethod).hasName(["putFirst", "putLast"]) and arg = 0
+  or
+  //java.util.Dictionary
+  method
+      .getDeclaringType()
+      .getSourceDeclaration()
+      .getASourceSupertype*()
+      .hasQualifiedName("java.util", "Dictionary") and
+  method.hasName("put") and
+  arg = 1
 }
 
 /**
  * Holds if `method` is a library method that returns tainted data if its
  * `arg`th argument is tainted.
  */
 private predicate taintPreservingArgumentToMethod(Method method, int arg) {
+  // java.util.Stack
+  method.(CollectionMethod).hasName("push") and arg = 0
+  or
   method.getDeclaringType().hasQualifiedName("java.util", "Collections") and
   (
     method