JS: Clean up taint step definitions

These are Unit types and so should be kept private as you can't use them for anything other than getting all taint steps of a certain type. Also factors out accesses to 'this'.
2026-05-02 04:05:14 +02:00 · 2021-08-23 14:59:23 +02:00
parent 5b0e26c814
commit 7dd65d8ac6
3 changed files with 44 additions and 39 deletions
--- a/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
@@ -328,16 +328,18 @@ module Vue {
      result = getAsClassComponent().getAnInstanceMember()
    }

+    /**
+     * Gets a reference to `this` inside the component, referring to an instance of the component.
+     */
+    DataFlow::SourceNode getASelfRef() {
+      result = getABoundFunction().getReceiver()
+    }
+
    pragma[noinline]
    private DataFlow::PropWrite getAPropertyValueWrite(string name) {
      result = getData().getALocalSource().getAPropertyWrite(name)
      or
-      result =
-        getABoundFunction()
-            .getALocalSource()
-            .(DataFlow::FunctionNode)
-            .getReceiver()
-            .getAPropertyWrite(name)
+      result = getASelfRef().getAPropertyWrite(name)
    }

    /**
@@ -547,20 +549,31 @@ module Vue {
    VueFile() { getExtension() = "vue" }
  }

+  pragma[nomagic]
+  private DataFlow::Node propStepPred(Component comp, string name) {
+    result = comp.getAPropertyValue(name)
+  }
+
+  pragma[nomagic]
+  private DataFlow::Node propStepSucc(Component comp, string name) {
+    result = comp.getASelfRef().getAPropertyRead(name)
+  }
+
  /**
   * A taint propagating data flow edge through a Vue instance property.
   */
-  class InstanceHeapStep extends TaintTracking::SharedTaintStep {
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(Component i, string name, DataFlow::FunctionNode bound |
-        bound.flowsTo(i.getABoundFunction()) and
-        not bound.getFunction() instanceof ArrowFunctionExpr and
-        succ = bound.getReceiver().getAPropertyRead(name) and
-        pred = i.getAPropertyValue(name)
+  private class PropStep extends TaintTracking::SharedTaintStep {
+    override predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(Component comp, string name |
+        pred = propStepPred(comp, name) and
+        succ = propStepSucc(comp, name)
      )
    }
  }

+  /** DEPRECATED. Do not use. */
+  deprecated class InstanceHeapStep = PropStep;
+
  /**
   * A Vue `v-html` attribute.
   */
@@ -585,11 +598,11 @@ module Vue {
   * of `inst = new Vue({ ..., data: { prop: source } })`, if the
   * `div` element is part of the template for `inst`.
   */
-  class VHtmlSourceWrite extends TaintTracking::SharedTaintStep {
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(Vue::Component component, string expr, VHtmlAttribute attr |
+  private class VHtmlAttributeStep extends TaintTracking::SharedTaintStep {
+    override predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(Component component, string expr, VHtmlAttribute attr |
        attr.getAttr().getRoot() =
-          component.getTemplateElement().(Vue::Template::HtmlElement).getElement() and
+          component.getTemplateElement().(Template::HtmlElement).getElement() and
        expr = attr.getAttr().getValue() and
        // only support for simple identifier expressions
        expr.regexpMatch("(?i)[a-z0-9_]+") and
@@ -599,6 +612,11 @@ module Vue {
    }
  }

+  /**
+   * DEPRECATED. Do not use.
+   */
+  deprecated class VHtmlSourceWrite = VHtmlAttributeStep;
+
  /*
   * Provides classes for working with Vue templates.
   */
--- a/javascript/ql/test/library-tests/frameworks/Vue/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/Vue/tests.expected
@@ -94,13 +94,13 @@ component
 | tst.js:107:12:109:2 | Vue.ext ... 23 }\\n}) |
 | tst.js:110:16:112:2 | new Vue ... base\\n}) |
 | tst.js:113:17:117:2 | base.ex ... 0\\n\\t}\\n}) |
-instance_heapStep
-| Unit | compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
-| Unit | tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
+viewComponentStep
+| compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
+| single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
+| single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
+| single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
+| single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
+| tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
 templateElement
 | compont-with-route.vue:1:1:3:11 | <template>...</> |
 | compont-with-route.vue:2:5:51:9 | <p>...</> |
@@ -126,13 +126,6 @@ templateElement
 | single-file-component-5.vue:2:5:18:9 | <p>...</> |
 | single-file-component-5.vue:4:1:16:9 | <script>...</> |
 | single-file-component-5.vue:17:1:18:8 | <style>...</> |
-vhtmlSourceWrite
-| Unit | compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
-| Unit | single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
-| Unit | tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
 xssSink
 | compont-with-route.vue:2:8:2:21 | v-html=dataA |
 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
--- a/javascript/ql/test/library-tests/frameworks/Vue/tests.ql
+++ b/javascript/ql/test/library-tests/frameworks/Vue/tests.ql
@@ -11,18 +11,12 @@ query predicate component_getOption(Vue::Component c, string name, DataFlow::Nod

 query predicate component(Vue::Component c) { any() }

-query predicate instance_heapStep(
-  Vue::InstanceHeapStep step, DataFlow::Node pred, DataFlow::Node succ
-) {
-  step.step(pred, succ)
+query predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
+  TaintTracking::viewComponentStep(pred, succ)
 }

 query predicate templateElement(Vue::Template::Element template) { any() }

-query predicate vhtmlSourceWrite(Vue::VHtmlSourceWrite w, DataFlow::Node pred, DataFlow::Node succ) {
-  w.step(pred, succ)
-}
-
 query predicate xssSink(DomBasedXss::Sink s) { any() }

 query RemoteFlowSource remoteFlowSource() { any() }