Skip to content

Commit 7dd65d8

Browse files
asgerfasgerf
committed
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'.
1 parent 5b0e26c commit 7dd65d8

File tree

3 files changed

+44
-39
lines changed

3 files changed

+44
-39
lines changed

‎javascript/ql/lib/semmle/javascript/frameworks/Vue.qll

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -328,16 +328,18 @@ module Vue {
328328
result = getAsClassComponent().getAnInstanceMember()
329329
}
330330

331+
/**
332+
* Gets a reference to `this` inside the component, referring to an instance of the component.
333+
*/
334+
DataFlow::SourceNode getASelfRef() {
335+
result = getABoundFunction().getReceiver()
336+
}
337+
331338
pragma[noinline]
332339
private DataFlow::PropWrite getAPropertyValueWrite(string name) {
333340
result = getData().getALocalSource().getAPropertyWrite(name)
334341
or
335-
result =
336-
getABoundFunction()
337-
.getALocalSource()
338-
.(DataFlow::FunctionNode)
339-
.getReceiver()
340-
.getAPropertyWrite(name)
342+
result = getASelfRef().getAPropertyWrite(name)
341343
}
342344

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

552+
pragma[nomagic]
553+
private DataFlow::Node propStepPred(Component comp, string name) {
554+
result = comp.getAPropertyValue(name)
555+
}
556+
557+
pragma[nomagic]
558+
private DataFlow::Node propStepSucc(Component comp, string name) {
559+
result = comp.getASelfRef().getAPropertyRead(name)
560+
}
561+
550562
/**
551563
* A taint propagating data flow edge through a Vue instance property.
552564
*/
553-
class InstanceHeapStep extends TaintTracking::SharedTaintStep {
554-
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
555-
exists(Component i, string name, DataFlow::FunctionNode bound |
556-
bound.flowsTo(i.getABoundFunction()) and
557-
not bound.getFunction() instanceof ArrowFunctionExpr and
558-
succ = bound.getReceiver().getAPropertyRead(name) and
559-
pred = i.getAPropertyValue(name)
565+
private class PropStep extends TaintTracking::SharedTaintStep {
566+
override predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
567+
exists(Component comp, string name |
568+
pred = propStepPred(comp, name) and
569+
succ = propStepSucc(comp, name)
560570
)
561571
}
562572
}
563573

574+
/** DEPRECATED. Do not use. */
575+
deprecated class InstanceHeapStep = PropStep;
576+
564577
/**
565578
* A Vue `v-html` attribute.
566579
*/
@@ -585,11 +598,11 @@ module Vue {
585598
* of `inst = new Vue({ ..., data: { prop: source } })`, if the
586599
* `div` element is part of the template for `inst`.
587600
*/
588-
class VHtmlSourceWrite extends TaintTracking::SharedTaintStep {
589-
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
590-
exists(Vue::Component component, string expr, VHtmlAttribute attr |
601+
private class VHtmlAttributeStep extends TaintTracking::SharedTaintStep {
602+
override predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
603+
exists(Component component, string expr, VHtmlAttribute attr |
591604
attr.getAttr().getRoot() =
592-
component.getTemplateElement().(Vue::Template::HtmlElement).getElement() and
605+
component.getTemplateElement().(Template::HtmlElement).getElement() and
593606
expr = attr.getAttr().getValue() and
594607
// only support for simple identifier expressions
595608
expr.regexpMatch("(?i)[a-z0-9_]+") and
@@ -599,6 +612,11 @@ module Vue {
599612
}
600613
}
601614

615+
/**
616+
* DEPRECATED. Do not use.
617+
*/
618+
deprecated class VHtmlSourceWrite = VHtmlAttributeStep;
619+
602620
/*
603621
* Provides classes for working with Vue templates.
604622
*/

‎javascript/ql/test/library-tests/frameworks/Vue/tests.expected

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ component
9494
| tst.js:107:12:109:2 | Vue.ext ... 23 }\\n}) |
9595
| tst.js:110:16:112:2 | new Vue ... base\\n}) |
9696
| tst.js:113:17:117:2 | base.ex ... 0\\n\\t}\\n}) |
97-
instance_heapStep
98-
| Unit | compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
99-
| Unit | single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
100-
| Unit | single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
101-
| Unit | single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
102-
| Unit | single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
103-
| Unit | tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
97+
viewComponentStep
98+
| compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
99+
| single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
100+
| single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
101+
| single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
102+
| single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
103+
| tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
104104
templateElement
105105
| compont-with-route.vue:1:1:3:11 | <template>...</> |
106106
| compont-with-route.vue:2:5:51:9 | <p>...</> |
@@ -126,13 +126,6 @@ templateElement
126126
| single-file-component-5.vue:2:5:18:9 | <p>...</> |
127127
| single-file-component-5.vue:4:1:16:9 | <script>...</> |
128128
| single-file-component-5.vue:17:1:18:8 | <style>...</> |
129-
vhtmlSourceWrite
130-
| Unit | compont-with-route.vue:31:14:31:34 | this.$r ... ery.foo | compont-with-route.vue:2:8:2:21 | v-html=dataA |
131-
| Unit | single-component-file-1.vue:6:40:6:41 | 42 | single-component-file-1.vue:2:8:2:21 | v-html=dataA |
132-
| Unit | single-file-component-3-script.js:4:37:4:38 | 42 | single-file-component-3.vue:2:8:2:21 | v-html=dataA |
133-
| Unit | single-file-component-4.vue:15:14:15:15 | 42 | single-file-component-4.vue:2:8:2:21 | v-html=dataA |
134-
| Unit | single-file-component-5.vue:13:14:13:15 | 42 | single-file-component-5.vue:2:8:2:21 | v-html=dataA |
135-
| Unit | tst.js:100:18:100:19 | 42 | tst.js:102:20:102:29 | this.dataA |
136129
xssSink
137130
| compont-with-route.vue:2:8:2:21 | v-html=dataA |
138131
| single-component-file-1.vue:2:8:2:21 | v-html=dataA |

‎javascript/ql/test/library-tests/frameworks/Vue/tests.ql

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,12 @@ query predicate component_getOption(Vue::Component c, string name, DataFlow::Nod
1111

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

14-
query predicate instance_heapStep(
15-
Vue::InstanceHeapStep step, DataFlow::Node pred, DataFlow::Node succ
16-
) {
17-
step.step(pred, succ)
14+
query predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
15+
TaintTracking::viewComponentStep(pred, succ)
1816
}
1917

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

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

2822
query RemoteFlowSource remoteFlowSource() { any() }

0 commit comments

Comments
 (0)