JS: Improve resolution of ArrayElement in API graphs

Author: asgerf

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -229,6 +229,23 @@ module API {
       result = this.getASuccessor(Label::unknownMember())
     }
 
+    /**
+     * Gets a node representing an arbitrary array element in the array represented by this node.
+     */
+    cached
+    Node getArrayElement() {
+      exists(string s, int n |
+        result = this.getMember(s) and
+        s.toInt() = n and
+        n.toString() = s and // ensure 's' is the canonical integer representation
+        n >= 0
+      )
+      or
+      result = this.getMember(DataFlow::PseudoProperties::arrayElement())
+      or
+      result = this.getUnknownMember()
+    }
+
     /**
      * Gets a node representing a member of this API component where the name of the member may
      * or may not be known statically.

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll

@@ -162,8 +162,8 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathTokenBase token) {
   token.getName() = "Awaited" and
   result = node.getPromised()
   or
-  token.getName() = "ArrayElement" and
-  result = node.getMember(DataFlow::PseudoProperties::arrayElement())
+  token.getName() = ["ArrayElement", "Element"] and
+  result = node.getArrayElement()
   or
   token.getName() = "Element" and
   result = node.getMember(DataFlow::PseudoProperties::arrayLikeElement())
@@ -172,11 +172,6 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathTokenBase token) {
   token.getName() = "MapValue" and
   result = node.getMember(DataFlow::PseudoProperties::mapValueAll())
   or
-  // Currently we need to include the "unknown member" for ArrayElement and Element since
-  // API graphs do not use store/load steps for arrays
-  token.getName() = ["ArrayElement", "Element"] and
-  result = node.getUnknownMember()
-  or
   token.getName() = "Parameter" and
   token.getAnArgument() = "this" and
   result = node.getReceiver()

javascript/ql/test/library-tests/frameworks/data/test.expected

@@ -81,6 +81,7 @@ taintFlow
 | test.js:272:6:272:40 | new MyS ... ource() | test.js:272:6:272:40 | new MyS ... ource() |
 | test.js:274:6:274:39 | testlib ... eName() | test.js:274:6:274:39 | testlib ... eName() |
 | test.js:277:8:277:31 | "danger ... .danger | test.js:277:8:277:31 | "danger ... .danger |
+| test.js:284:8:284:16 | source[0] | test.js:284:8:284:16 | source[0] |
 isSink
 | test.js:54:18:54:25 | source() | test-sink |
 | test.js:55:22:55:29 | source() | test-sink |

javascript/ql/test/library-tests/frameworks/data/test.js

@@ -281,5 +281,5 @@ function dangerConstant() {
 
 function arraySource() {
   const source = testlib.getSourceArray();
-  sink(source[0]); // NOT OK [INCONSISTENCY]
+  sink(source[0]); // NOT OK
 }