JS: Handle inheritance

Author: asgerf

javascript/ql/src/semmle/javascript/dataflow/internal/CallGraphs.qll

@@ -163,16 +163,36 @@ module CallGraph {
     )
   }
 
+  /** Holds if a property setter named `name` exists in a class. */
+  private predicate isSetterName(string name) {
+    exists(any(DataFlow::ClassNode cls).getInstanceMember(name, DataFlow::MemberKind::setter()))
+  }
+
+  /**
+   * Gets a property write that assigns to the property `name` on an instance of this class,
+   * and `name` is the name of a property setter.
+   */
+  private DataFlow::PropWrite getAnInstanceMemberAssignment(DataFlow::ClassNode cls, string name) {
+    isSetterName(name) and // restrict size of predicate
+    result = cls.getAnInstanceReference().getAPropertyWrite(name)
+    or
+    exists(DataFlow::ClassNode subclass |
+      result = getAnInstanceMemberAssignment(subclass, name) and
+      not exists(subclass.getInstanceMember(name, DataFlow::MemberKind::setter())) and
+      cls = subclass.getADirectSuperClass()
+    )
+  }
+
   /**
    * Gets a getter or setter invoked as a result of the given property access.
    */
   cached
   DataFlow::FunctionNode getAnAccessorCallee(DataFlow::PropRef ref) {
     exists(DataFlow::ClassNode cls, string name |
-      ref = cls.getAnInstanceReference().getAPropertyRead(name) and
+      ref = cls.getAnInstanceMemberAccess(name) and
       result = cls.getInstanceMember(name, DataFlow::MemberKind::getter())
       or
-      ref = cls.getAnInstanceReference().getAPropertyWrite(name) and
+      ref = getAnInstanceMemberAssignment(cls, name) and
       result = cls.getInstanceMember(name, DataFlow::MemberKind::setter())
     )
     or

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -72,6 +72,10 @@ typeInferenceMismatch
 | getters-and-setters.js:47:23:47:30 | source() | getters-and-setters.js:45:14:45:16 | c.x |
 | getters-and-setters.js:60:20:60:27 | source() | getters-and-setters.js:66:10:66:14 | obj.x |
 | getters-and-setters.js:67:13:67:20 | source() | getters-and-setters.js:63:18:63:22 | value |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:88:10:88:18 | new C().x |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:92:14:92:16 | c.x |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:100:10:100:22 | getX(new C()) |
+| getters-and-setters.js:89:17:89:24 | source() | getters-and-setters.js:82:18:82:22 | value |
 | importedReactComponent.jsx:4:40:4:47 | source() | exportedReactComponent.jsx:2:10:2:19 | props.text |
 | indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
 | json-stringify.js:2:16:2:23 | source() | json-stringify.js:5:8:5:29 | JSON.st ... source) |

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

@@ -47,6 +47,10 @@
 | getters-and-setters.js:47:23:47:30 | source() | getters-and-setters.js:45:14:45:16 | c.x |
 | getters-and-setters.js:60:20:60:27 | source() | getters-and-setters.js:66:10:66:14 | obj.x |
 | getters-and-setters.js:67:13:67:20 | source() | getters-and-setters.js:63:18:63:22 | value |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:88:10:88:18 | new C().x |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:92:14:92:16 | c.x |
+| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:100:10:100:22 | getX(new C()) |
+| getters-and-setters.js:89:17:89:24 | source() | getters-and-setters.js:82:18:82:22 | value |
 | indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
 | indexOf.js:4:11:4:18 | source() | indexOf.js:13:10:13:10 | x |
 | nested-props.js:4:13:4:20 | source() | nested-props.js:5:10:5:14 | obj.x |

javascript/ql/test/library-tests/TaintTracking/getters-and-setters.js

@@ -72,3 +72,31 @@ function testFlowThroughObjectLiteralAccessors() {
     indirection(obj);
     indirection(null);
 }
+
+function testFlowThroughSubclass() {
+    class Base {
+        get x() {
+            return source();
+        }
+        set y(value) {
+            sink(value); // NOT OK
+        }
+    };
+    class C extends Base {
+    }
+
+    sink(new C().x); // NOT OK
+    new C().y = source();
+
+    function indirection(c) {
+        sink(c.x); // NOT OK
+    }
+    indirection(new C());
+    indirection(null);
+
+    function getX(c) {
+        return c.x;
+    }
+    sink(getX(new C())); // NOT OK - but not flagged
+    getX(null);
+}