Skip to content

Commit b015c88

Browse files
asgerfasgerf
committed
JS: Add view-component-input threat model
1 parent 01f7d45 commit b015c88

19 files changed

+309
-47
lines changed

‎javascript/ql/lib/semmle/javascript/ViewComponentInput.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Provides a classes and predicates for contributing to the `view-component-input` threat model.
3+
*/
4+
5+
private import javascript
6+
7+
/**
8+
* An input to a view component, such as React props.
9+
*/
10+
abstract class ViewComponentInput extends ThreatModelSource::Range {
11+
final override string getThreatModel() { result = "view-component-input" }
12+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
88
private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations
99
private import semmle.javascript.DynamicPropertyAccess
1010
private import semmle.javascript.dataflow.internal.PreCallGraphStep
11+
private import semmle.javascript.ViewComponentInput
1112

1213
/**
1314
* Provides classes for working with Angular (also known as Angular 2.x) applications.
@@ -575,4 +576,17 @@ module Angular2 {
575576
)
576577
}
577578
}
579+
580+
private class InputFieldAsViewComponentInput extends ViewComponentInput {
581+
InputFieldAsViewComponentInput() {
582+
this =
583+
API::moduleImport("@angular/core")
584+
.getMember("Input")
585+
.getReturn()
586+
.getADecoratedMember()
587+
.asSource()
588+
}
589+
590+
override string getSourceType() { result = "Angular component input field" }
591+
}
578592
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import javascript
66
private import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
77
private import semmle.javascript.dataflow.internal.PreCallGraphStep
8+
private import semmle.javascript.ViewComponentInput
89

910
/**
1011
* Gets a reference to the 'React' object.
@@ -868,3 +869,9 @@ private class PropsFlowStep extends PreCallGraphStep {
868869
)
869870
}
870871
}
872+
873+
private class ReactPropAsViewComponentInput extends ViewComponentInput {
874+
ReactPropAsViewComponentInput() { this = any(ReactComponent c).getADirectPropsAccess() }
875+
876+
override string getSourceType() { result = "React props" }
877+
}

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

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import javascript
6+
import semmle.javascript.ViewComponentInput
67

78
module Vue {
89
/** The global variable `Vue`, as an API graph entry point. */
@@ -85,17 +86,16 @@ module Vue {
8586
* A class with a `@Component` decorator, making it usable as an "options" object in Vue.
8687
*/
8788
class ClassComponent extends DataFlow::ClassNode {
89+
private ClassDefinition cls;
8890
DataFlow::Node decorator;
8991

9092
ClassComponent() {
91-
exists(ClassDefinition cls |
92-
this = cls.flow() and
93-
cls.getADecorator().getExpression() = decorator.asExpr() and
94-
(
95-
componentDecorator().flowsTo(decorator)
96-
or
97-
componentDecorator().getACall() = decorator
98-
)
93+
this = cls.flow() and
94+
cls.getADecorator().getExpression() = decorator.asExpr() and
95+
(
96+
componentDecorator().flowsTo(decorator)
97+
or
98+
componentDecorator().getACall() = decorator
9999
)
100100
}
101101

@@ -105,6 +105,9 @@ module Vue {
105105
* These options correspond to the options one would pass to `new Vue({...})` or similar.
106106
*/
107107
API::Node getDecoratorOptions() { result = decorator.(API::CallNode).getParameter(0) }
108+
109+
/** Gets the AST node for the class definition. */
110+
ClassDefinition getClassDefinition() { result = cls }
108111
}
109112

110113
private string memberKindVerb(DataFlow::MemberKind kind) {
@@ -460,6 +463,12 @@ module Vue {
460463

461464
SingleFileComponent() { this = MkSingleFileComponent(file) }
462465

466+
/** Gets a call to `defineProps` in this component. */
467+
DataFlow::CallNode getDefinePropsCall() {
468+
result = DataFlow::globalVarRef("defineProps").getACall() and
469+
result.getFile() = file
470+
}
471+
463472
override Template::Element getTemplateElement() {
464473
exists(HTML::Element e | result.(Template::HtmlElement).getElement() = e |
465474
e.getFile() = file and
@@ -697,4 +706,68 @@ module Vue {
697706

698707
override ClientSideRemoteFlowKind getKind() { result = kind }
699708
}
709+
710+
/**
711+
* Holds if the given type annotation indicates a value that is not typically considered taintable.
712+
*/
713+
private predicate isSafeType(TypeAnnotation type) {
714+
type.isBooleany() or
715+
type.isNumbery() or
716+
type.isRawFunction() or
717+
type instanceof FunctionTypeExpr
718+
}
719+
720+
/**
721+
* Holds if the given field has a type that indicates that is can not contain a taintable value.
722+
*/
723+
private predicate isSafeField(FieldDeclaration field) { isSafeType(field.getTypeAnnotation()) }
724+
725+
private DataFlow::Node getPropSpec(Component component) {
726+
result = component.getOption("props")
727+
or
728+
result = component.(SingleFileComponent).getDefinePropsCall().getArgument(0)
729+
}
730+
731+
/**
732+
* Holds if `component` has an input prop with the given name, that is of a taintable type.
733+
*/
734+
private predicate hasTaintableProp(Component component, string name) {
735+
exists(DataFlow::SourceNode spec | spec = getPropSpec(component).getALocalSource() |
736+
spec.(DataFlow::ArrayCreationNode).getAnElement().getStringValue() = name
737+
or
738+
exists(DataFlow::PropWrite write |
739+
write = spec.getAPropertyWrite(name) and
740+
not DataFlow::globalVarRef(["Number", "Boolean"]).flowsTo(write.getRhs())
741+
)
742+
)
743+
or
744+
exists(FieldDeclaration field |
745+
field = component.getAsClassComponent().getClassDefinition().getField(name) and
746+
DataFlow::moduleMember("vue-property-decorator", "Prop")
747+
.getACall()
748+
.flowsToExpr(field.getADecorator().getExpression()) and
749+
not isSafeField(field)
750+
)
751+
or
752+
// defineProps() can be called with only type arguments and then the Vue compiler will
753+
// infer the prop types.
754+
exists(CallExpr call, FieldDeclaration field |
755+
call = component.(SingleFileComponent).getDefinePropsCall().asExpr() and
756+
field = call.getTypeArgument(0).(InterfaceTypeExpr).getMember(name) and
757+
not isSafeField(field)
758+
)
759+
}
760+
761+
private class PropAsViewComponentInput extends ViewComponentInput {
762+
PropAsViewComponentInput() {
763+
exists(Component component, string name | hasTaintableProp(component, name) |
764+
this = component.getAnInstanceRef().getAPropertyRead(name)
765+
or
766+
// defineProps() returns the props
767+
this = component.(SingleFileComponent).getDefinePropsCall().getAPropertyRead(name)
768+
)
769+
}
770+
771+
override string getSourceType() { result = "Vue prop" }
772+
}
700773
}

‎javascript/ql/test/library-tests/frameworks/Angular2/sink.component.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
import { Component } from "@angular/core";
1+
import { Component, Input } from "@angular/core";
22
import { DomSanitizer } from '@angular/platform-browser';
33

44
@Component({
55
selector: "sink-component",
66
template: "not important"
77
})
88
export class SinkComponent {
9-
sink1: string;
10-
sink2: string;
11-
sink3: string;
12-
sink4: string;
13-
sink5: string;
14-
sink6: string;
15-
sink7: string;
16-
sink8: string;
17-
sink9: string;
9+
@Input() sink1: string;
10+
@Input() sink2: string;
11+
@Input() sink3: string;
12+
@Input() sink4: string;
13+
@Input() sink5: string;
14+
@Input() sink6: string;
15+
@Input() sink7: string;
16+
@Input() sink8: string;
17+
@Input() sink9: string;
1818

19-
constructor(private sanitizer: DomSanitizer) {}
19+
constructor(private sanitizer: DomSanitizer) { }
2020

2121
foo() {
2222
this.sanitizer.bypassSecurityTrustHtml(this.sink1);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,13 @@ taintFlow
3636
| source.component.ts:16:33:16:40 | source() | sink.component.ts:22:48:22:57 | this.sink1 |
3737
testAttrSourceLocation
3838
| inline.component.ts:8:43:8:60 | [testAttr]=taint | inline.component.ts:8:55:8:59 | <toplevel> |
39+
threatModelSource
40+
| sink.component.ts:22:48:22:57 | this.sink1 | view-component-input |
41+
| sink.component.ts:23:48:23:57 | this.sink2 | view-component-input |
42+
| sink.component.ts:24:48:24:57 | this.sink3 | view-component-input |
43+
| sink.component.ts:25:48:25:57 | this.sink4 | view-component-input |
44+
| sink.component.ts:26:48:26:57 | this.sink5 | view-component-input |
45+
| sink.component.ts:27:48:27:57 | this.sink6 | view-component-input |
46+
| sink.component.ts:28:48:28:57 | this.sink7 | view-component-input |
47+
| sink.component.ts:29:48:29:57 | this.sink8 | view-component-input |
48+
| sink.component.ts:30:48:30:57 | this.sink9 | view-component-input |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,7 @@ deprecated class LegacyConfig extends TaintTracking::Configuration {
4242
}
4343

4444
deprecated import utils.test.LegacyDataFlowDiff::DataFlowDiff<TestFlow, LegacyConfig>
45+
46+
query predicate threatModelSource(ThreatModelSource source, string kind) {
47+
kind = source.getThreatModel()
48+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,3 +318,28 @@ test_JsxName_this
318318
| thisAccesses.js:61:19:61:41 | <this.t ... s.this> | thisAccesses.js:61:20:61:23 | this |
319319
locationSource
320320
| importedComponent.jsx:3:32:3:39 | location |
321+
threatModelSource
322+
| es5.js:4:24:4:33 | this.props | view-component-input |
323+
| es5.js:20:24:20:33 | this.props | view-component-input |
324+
| es6.js:1:37:1:36 | args | view-component-input |
325+
| es6.js:3:24:3:33 | this.props | view-component-input |
326+
| exportedComponent.jsx:1:29:1:33 | props | view-component-input |
327+
| importedComponent.jsx:3:24:3:40 | {color, location} | view-component-input |
328+
| importedComponent.jsx:3:32:3:39 | location | remote |
329+
| namedImport.js:3:27:3:26 | args | view-component-input |
330+
| namedImport.js:5:19:5:18 | args | view-component-input |
331+
| plainfn.js:1:16:1:20 | props | view-component-input |
332+
| plainfn.js:5:17:5:21 | props | view-component-input |
333+
| plainfn.js:9:17:9:21 | props | view-component-input |
334+
| plainfn.js:20:28:20:32 | props | view-component-input |
335+
| preact.js:1:38:1:37 | args | view-component-input |
336+
| preact.js:2:12:2:16 | props | view-component-input |
337+
| preact.js:9:38:9:37 | args | view-component-input |
338+
| probably-a-component.js:1:31:1:30 | args | view-component-input |
339+
| probably-a-component.js:3:9:3:18 | this.props | view-component-input |
340+
| props.js:2:37:2:36 | args | view-component-input |
341+
| props.js:26:16:26:20 | props | view-component-input |
342+
| rare-lifecycle-methods.js:1:33:1:32 | args | view-component-input |
343+
| statePropertyWrites.js:38:24:38:33 | this.props | view-component-input |
344+
| thisAccesses.js:31:12:31:16 | props | view-component-input |
345+
| thisAccesses.js:48:18:48:18 | y | view-component-input |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,7 @@ import ReactComponent_getAPropRead
1111
import ReactName
1212

1313
query DataFlow::SourceNode locationSource() { result = DOM::locationSource() }
14+
15+
query predicate threatModelSource(ThreatModelSource source, string kind) {
16+
kind = source.getThreatModel()
17+
}

‎javascript/ql/test/library-tests/frameworks/Vue/single-component-file-1.vue‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
</template>
44
<script>
55
export default {
6-
data: function() { return { dataA: 42 } }
6+
props: ['input'],
7+
data: function() { return { dataA: 42 + this.input } }
78
}
89
</script>
910
<style>

0 commit comments

Comments
 (0)