Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS: Add view-component-input threat model #18466

Merged
merged 10 commits into from
Jan 24, 2025
12 changes: 12 additions & 0 deletions javascript/ql/lib/semmle/javascript/ViewComponentInput.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Provides a classes and predicates for contributing to the `view-component-input` threat model.
*/

private import javascript

/**
* An input to a view component, such as React props.
*/
abstract class ViewComponentInput extends ThreatModelSource::Range {
final override string getThreatModel() { result = "view-component-input" }
}
14 changes: 14 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations
private import semmle.javascript.DynamicPropertyAccess
private import semmle.javascript.dataflow.internal.PreCallGraphStep
private import semmle.javascript.ViewComponentInput

/**
* Provides classes for working with Angular (also known as Angular 2.x) applications.
Expand Down Expand Up @@ -575,4 +576,17 @@ module Angular2 {
)
}
}

private class InputFieldAsViewComponentInput extends ViewComponentInput {
InputFieldAsViewComponentInput() {
this =
API::moduleImport("@angular/core")
.getMember("Input")
.getReturn()
.getADecoratedMember()
.asSource()
}

override string getSourceType() { result = "Angular component input field" }
}
}
7 changes: 7 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/React.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import javascript
private import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
private import semmle.javascript.dataflow.internal.PreCallGraphStep
private import semmle.javascript.ViewComponentInput

/**
* Gets a reference to the 'React' object.
Expand Down Expand Up @@ -868,3 +869,9 @@ private class PropsFlowStep extends PreCallGraphStep {
)
}
}

private class ReactPropAsViewComponentInput extends ViewComponentInput {
ReactPropAsViewComponentInput() { this = any(ReactComponent c).getADirectPropsAccess() }

override string getSourceType() { result = "React props" }
}
89 changes: 81 additions & 8 deletions javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

import javascript
import semmle.javascript.ViewComponentInput

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

ClassComponent() {
exists(ClassDefinition cls |
this = cls.flow() and
cls.getADecorator().getExpression() = decorator.asExpr() and
(
componentDecorator().flowsTo(decorator)
or
componentDecorator().getACall() = decorator
)
this = cls.flow() and
cls.getADecorator().getExpression() = decorator.asExpr() and
(
componentDecorator().flowsTo(decorator)
or
componentDecorator().getACall() = decorator
)
}

Expand All @@ -105,6 +105,9 @@ module Vue {
* These options correspond to the options one would pass to `new Vue({...})` or similar.
*/
API::Node getDecoratorOptions() { result = decorator.(API::CallNode).getParameter(0) }

/** Gets the AST node for the class definition. */
ClassDefinition getClassDefinition() { result = cls }
}

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

SingleFileComponent() { this = MkSingleFileComponent(file) }

/** Gets a call to `defineProps` in this component. */
DataFlow::CallNode getDefinePropsCall() {
result = DataFlow::globalVarRef("defineProps").getACall() and
result.getFile() = file
}

override Template::Element getTemplateElement() {
exists(HTML::Element e | result.(Template::HtmlElement).getElement() = e |
e.getFile() = file and
Expand Down Expand Up @@ -697,4 +706,68 @@ module Vue {

override ClientSideRemoteFlowKind getKind() { result = kind }
}

/**
* Holds if the given type annotation indicates a value that is not typically considered taintable.
*/
private predicate isSafeType(TypeAnnotation type) {
type.isBooleany() or
type.isNumbery() or
type.isRawFunction() or
type instanceof FunctionTypeExpr
}

/**
* Holds if the given field has a type that indicates that is can not contain a taintable value.
*/
private predicate isSafeField(FieldDeclaration field) { isSafeType(field.getTypeAnnotation()) }

private DataFlow::Node getPropSpec(Component component) {
result = component.getOption("props")
or
result = component.(SingleFileComponent).getDefinePropsCall().getArgument(0)
}

/**
* Holds if `component` has an input prop with the given name, that is of a taintable type.
*/
private predicate hasTaintableProp(Component component, string name) {
exists(DataFlow::SourceNode spec | spec = getPropSpec(component).getALocalSource() |
spec.(DataFlow::ArrayCreationNode).getAnElement().getStringValue() = name
or
exists(DataFlow::PropWrite write |
write = spec.getAPropertyWrite(name) and
not DataFlow::globalVarRef(["Number", "Boolean"]).flowsTo(write.getRhs())
)
)
or
exists(FieldDeclaration field |
field = component.getAsClassComponent().getClassDefinition().getField(name) and
DataFlow::moduleMember("vue-property-decorator", "Prop")
.getACall()
.flowsToExpr(field.getADecorator().getExpression()) and
not isSafeField(field)
)
or
// defineProps() can be called with only type arguments and then the Vue compiler will
// infer the prop types.
exists(CallExpr call, FieldDeclaration field |
call = component.(SingleFileComponent).getDefinePropsCall().asExpr() and
field = call.getTypeArgument(0).(InterfaceTypeExpr).getMember(name) and
not isSafeField(field)
)
}

private class PropAsViewComponentInput extends ViewComponentInput {
PropAsViewComponentInput() {
exists(Component component, string name | hasTaintableProp(component, name) |
this = component.getAnInstanceRef().getAPropertyRead(name)
or
// defineProps() returns the props
this = component.(SingleFileComponent).getDefinePropsCall().getAPropertyRead(name)
)
}

override string getSourceType() { result = "Vue prop" }
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { Component } from "@angular/core";
import { Component, Input } from "@angular/core";
import { DomSanitizer } from '@angular/platform-browser';

@Component({
selector: "sink-component",
template: "not important"
})
export class SinkComponent {
sink1: string;
sink2: string;
sink3: string;
sink4: string;
sink5: string;
sink6: string;
sink7: string;
sink8: string;
sink9: string;
@Input() sink1: string;
@Input() sink2: string;
@Input() sink3: string;
@Input() sink4: string;
@Input() sink5: string;
@Input() sink6: string;
@Input() sink7: string;
@Input() sink8: string;
@Input() sink9: string;
Comment on lines +9 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these are named "sink", but they're sources, right?

Copy link
Contributor Author

@asgerf asgerf Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're like React props, they're just passed in from the instantiation site. When I initially wrote this test long ago, I had omitted the @Input() decorator on these fields, which you're supposed to put on such input fields in Angular (not sure if they're strictly required though).

If the threat model is enabled they become sources (in addition to having incoming flow from instantiation sites). The unit test now reports which nodes are associated with this threat model, but the threat model is not actually enabled, so the data flow test doesn't treat them as taint sources.


constructor(private sanitizer: DomSanitizer) {}
constructor(private sanitizer: DomSanitizer) { }

foo() {
this.sanitizer.bypassSecurityTrustHtml(this.sink1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,13 @@ taintFlow
| source.component.ts:16:33:16:40 | source() | sink.component.ts:22:48:22:57 | this.sink1 |
testAttrSourceLocation
| inline.component.ts:8:43:8:60 | [testAttr]=taint | inline.component.ts:8:55:8:59 | <toplevel> |
threatModelSource
| sink.component.ts:22:48:22:57 | this.sink1 | view-component-input |
| sink.component.ts:23:48:23:57 | this.sink2 | view-component-input |
| sink.component.ts:24:48:24:57 | this.sink3 | view-component-input |
| sink.component.ts:25:48:25:57 | this.sink4 | view-component-input |
| sink.component.ts:26:48:26:57 | this.sink5 | view-component-input |
| sink.component.ts:27:48:27:57 | this.sink6 | view-component-input |
| sink.component.ts:28:48:28:57 | this.sink7 | view-component-input |
| sink.component.ts:29:48:29:57 | this.sink8 | view-component-input |
| sink.component.ts:30:48:30:57 | this.sink9 | view-component-input |
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,7 @@ deprecated class LegacyConfig extends TaintTracking::Configuration {
}

deprecated import utils.test.LegacyDataFlowDiff::DataFlowDiff<TestFlow, LegacyConfig>

query predicate threatModelSource(ThreatModelSource source, string kind) {
kind = source.getThreatModel()
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,28 @@ test_JsxName_this
| thisAccesses.js:61:19:61:41 | <this.t ... s.this> | thisAccesses.js:61:20:61:23 | this |
locationSource
| importedComponent.jsx:3:32:3:39 | location |
threatModelSource
| es5.js:4:24:4:33 | this.props | view-component-input |
| es5.js:20:24:20:33 | this.props | view-component-input |
| es6.js:1:37:1:36 | args | view-component-input |
| es6.js:3:24:3:33 | this.props | view-component-input |
| exportedComponent.jsx:1:29:1:33 | props | view-component-input |
| importedComponent.jsx:3:24:3:40 | {color, location} | view-component-input |
| importedComponent.jsx:3:32:3:39 | location | remote |
| namedImport.js:3:27:3:26 | args | view-component-input |
| namedImport.js:5:19:5:18 | args | view-component-input |
| plainfn.js:1:16:1:20 | props | view-component-input |
| plainfn.js:5:17:5:21 | props | view-component-input |
| plainfn.js:9:17:9:21 | props | view-component-input |
| plainfn.js:20:28:20:32 | props | view-component-input |
| preact.js:1:38:1:37 | args | view-component-input |
| preact.js:2:12:2:16 | props | view-component-input |
| preact.js:9:38:9:37 | args | view-component-input |
| probably-a-component.js:1:31:1:30 | args | view-component-input |
| probably-a-component.js:3:9:3:18 | this.props | view-component-input |
| props.js:2:37:2:36 | args | view-component-input |
| props.js:26:16:26:20 | props | view-component-input |
| rare-lifecycle-methods.js:1:33:1:32 | args | view-component-input |
| statePropertyWrites.js:38:24:38:33 | this.props | view-component-input |
| thisAccesses.js:31:12:31:16 | props | view-component-input |
| thisAccesses.js:48:18:48:18 | y | view-component-input |
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ import ReactComponent_getAPropRead
import ReactName

query DataFlow::SourceNode locationSource() { result = DOM::locationSource() }

query predicate threatModelSource(ThreatModelSource source, string kind) {
kind = source.getThreatModel()
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
</template>
<script>
export default {
data: function() { return { dataA: 42 } }
props: ['input'],
data: function() { return { dataA: 42 + this.input } }
}
</script>
<style>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
<script>
var x = require('x');
module.exports = { // not properly detected by the module system yet
data: function() { return { dataA: 42 } }
props: ['input'],
data: function() { return { dataA: 42 + this.input } }
}
</script>
<style>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var x = require('x');

module.exports = {
data: function() { return { dataA: 42 } }
props: ['input'],
data: function() { return { dataA: 42 + this.input } }
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,22 @@
<script>
import Vue from 'vue'
import Component from 'vue-class-component'
import { Prop } from 'vue-property-decorator'

@Component({
render: (h) => { }
})
export default class MyComponent extends Vue {
message: string = 'Hello!'
@Prop() input: string;

get dataA() {
return 42;
}

get dataB() {
return this.input;
}
}
</script>
<style>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@
<script>
import Vue from 'vue'
import Component from 'vue-class-component'
import { Prop } from 'vue-property-decorator'

@Component
export default class MyComponent extends Vue {
message: string = 'Hello!'
@Prop() input: string;

get dataA() {
return 42;
}

get dataB() {
return this.input;
}
}
</script>
<style>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<template>
<p v-html="input"/>
</template>
<script setup>
const { input } = defineProps(['input']);
</script>
<style>
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<template>
<p v-html="input"/>
</template>
<script setup>
const { input, numericInput, booleanInput } = defineProps({
input: String,
numericInput: Number,
booleanInput: Boolean,
});
</script>
<style>
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<template>
<p v-html="input"/>
</template>
<script setup lang="ts">
const { input, numericInput, booleanInput } = defineProps<{
input: string,
numericInput: number,
booleanInput: boolean,
}>();
</script>
<style>
</style>
Loading