From b306571d52afbdce9d89471b7561d2f99e6aec55 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Tue, 28 Jan 2020 10:22:04 +0000 Subject: [PATCH] JS: Type-track react component factories --- .../semmle/javascript/frameworks/React.qll | 41 +++++++++++++++++-- .../TaintTracking/BasicTaintTracking.expected | 1 + .../TaintTracking/exportedReactComponent.jsx | 4 ++ .../TaintTracking/importedReactComponent.jsx | 5 +++ .../frameworks/ReactJS/exportedComponent.jsx | 3 ++ .../frameworks/ReactJS/importedComponent.jsx | 5 +++ .../frameworks/ReactJS/tests.expected | 6 +++ 7 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 javascript/ql/test/library-tests/TaintTracking/exportedReactComponent.jsx create mode 100644 javascript/ql/test/library-tests/TaintTracking/importedReactComponent.jsx create mode 100644 javascript/ql/test/library-tests/frameworks/ReactJS/exportedComponent.jsx create mode 100644 javascript/ql/test/library-tests/frameworks/ReactJS/importedComponent.jsx diff --git a/javascript/ql/src/semmle/javascript/frameworks/React.qll b/javascript/ql/src/semmle/javascript/frameworks/React.qll index e94f3c6bacd7..a8c1793a5cc6 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/React.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/React.qll @@ -38,10 +38,15 @@ abstract class ReactComponent extends ASTNode { abstract AbstractValue getAbstractComponent(); /** - * Gets the value that instantiates this component when invoked. + * Gets the function that instantiates this component when invoked. */ abstract DataFlow::SourceNode getComponentCreatorSource(); + /** + * Gets a reference to the function that instantiates this component when invoked. + */ + abstract DataFlow::SourceNode getAComponentCreatorReference(); + /** * Gets a reference to this component. */ @@ -181,7 +186,7 @@ abstract class ReactComponent extends ASTNode { * constructor of this component. */ DataFlow::SourceNode getACandidatePropsSource() { - result.flowsTo(getComponentCreatorSource().getAnInvocation().getArgument(0)) + result.flowsTo(getAComponentCreatorReference().getAnInvocation().getArgument(0)) or result = getADefaultPropsSource() or @@ -269,6 +274,19 @@ class FunctionalComponent extends ReactComponent, Function { override AbstractValue getAbstractComponent() { result = AbstractInstance::of(this) } + private DataFlow::SourceNode getAComponentCreatorReference(DataFlow::TypeTracker t) { + t.start() and + result = DataFlow::valueNode(this) + or + exists(DataFlow::TypeTracker t2 | + result = getAComponentCreatorReference(t2).track(t2, t) + ) + } + + override DataFlow::SourceNode getAComponentCreatorReference() { + result = getAComponentCreatorReference(DataFlow::TypeTracker::end()) + } + override DataFlow::SourceNode getComponentCreatorSource() { result = DataFlow::valueNode(this) } override DataFlow::SourceNode getADefaultPropsSource() { @@ -302,6 +320,10 @@ abstract private class SharedReactPreactClassComponent extends ReactComponent, C override AbstractValue getAbstractComponent() { result = AbstractInstance::of(this) } + override DataFlow::SourceNode getAComponentCreatorReference() { + result = DataFlow::valueNode(this).(DataFlow::ClassNode).getAClassReference() + } + override DataFlow::SourceNode getComponentCreatorSource() { result = DataFlow::valueNode(this) } override DataFlow::SourceNode getACandidateStateSource() { @@ -420,6 +442,19 @@ class ES5Component extends ReactComponent, ObjectExpr { override AbstractValue getAbstractComponent() { result = TAbstractObjectLiteral(this) } + private DataFlow::SourceNode getAComponentCreatorReference(DataFlow::TypeTracker t) { + t.start() and + result = create + or + exists(DataFlow::TypeTracker t2 | + result = getAComponentCreatorReference(t2).track(t2, t) + ) + } + + override DataFlow::SourceNode getAComponentCreatorReference() { + result = getAComponentCreatorReference(DataFlow::TypeTracker::end()) + } + override DataFlow::SourceNode getComponentCreatorSource() { result = create } override DataFlow::SourceNode getACandidateStateSource() { @@ -517,7 +552,7 @@ private class AnalyzedThisInBoundCallback extends AnalyzedNode, DataFlow::ThisNo private class ReactJSXElement extends JSXElement { ReactComponent component; - ReactJSXElement() { component.getComponentCreatorSource().flowsToExpr(getNameExpr()) } + ReactJSXElement() { component.getAComponentCreatorReference().flowsToExpr(getNameExpr()) } /** * Gets the component this element instantiates. diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected index da4105238bc3..22c9a6c4576e 100644 --- a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected +++ b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected @@ -59,6 +59,7 @@ typeInferenceMismatch | exceptions.js:144:9:144:16 | source() | exceptions.js:132:8:132:27 | returnThrownSource() | | exceptions.js:150:13:150:20 | source() | exceptions.js:153:10:153:10 | e | | exceptions.js:158:13:158:20 | source() | exceptions.js:161:10:161:10 | e | +| 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 | | partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x | | partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y | diff --git a/javascript/ql/test/library-tests/TaintTracking/exportedReactComponent.jsx b/javascript/ql/test/library-tests/TaintTracking/exportedReactComponent.jsx new file mode 100644 index 000000000000..6489e09a1e51 --- /dev/null +++ b/javascript/ql/test/library-tests/TaintTracking/exportedReactComponent.jsx @@ -0,0 +1,4 @@ +export function UnsafeReactComponent(props) { + sink(props.text); + return
+} diff --git a/javascript/ql/test/library-tests/TaintTracking/importedReactComponent.jsx b/javascript/ql/test/library-tests/TaintTracking/importedReactComponent.jsx new file mode 100644 index 000000000000..5bef649e05df --- /dev/null +++ b/javascript/ql/test/library-tests/TaintTracking/importedReactComponent.jsx @@ -0,0 +1,5 @@ +import { UnsafeReactComponent } from "./exportedReactComponent"; + +export function render() { + return +} diff --git a/javascript/ql/test/library-tests/frameworks/ReactJS/exportedComponent.jsx b/javascript/ql/test/library-tests/frameworks/ReactJS/exportedComponent.jsx new file mode 100644 index 000000000000..4335b4bc3081 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/ReactJS/exportedComponent.jsx @@ -0,0 +1,3 @@ +export function MyComponent(props) { + return
+} diff --git a/javascript/ql/test/library-tests/frameworks/ReactJS/importedComponent.jsx b/javascript/ql/test/library-tests/frameworks/ReactJS/importedComponent.jsx new file mode 100644 index 000000000000..edc333528a7f --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/ReactJS/importedComponent.jsx @@ -0,0 +1,5 @@ +import { MyComponent } from "./exportedComponent"; + +export function render(color) { + return +} diff --git a/javascript/ql/test/library-tests/frameworks/ReactJS/tests.expected b/javascript/ql/test/library-tests/frameworks/ReactJS/tests.expected index 9b737cf1ea36..93e4d0d258a4 100644 --- a/javascript/ql/test/library-tests/frameworks/ReactJS/tests.expected +++ b/javascript/ql/test/library-tests/frameworks/ReactJS/tests.expected @@ -15,6 +15,7 @@ test_ReactComponent_getInstanceMethod | es5.js:1:31:11:1 | {\\n dis ... ;\\n }\\n} | render | es5.js:3:11:5:3 | functio ... v>;\\n } | | es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | render | es5.js:19:11:21:3 | functio ... 1>;\\n } | | es6.js:1:1:8:1 | class H ... ;\\n }\\n} | render | es6.js:2:9:4:3 | () {\\n ... v>;\\n } | +| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | render | exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | | plainfn.js:1:1:3:1 | functio ... div>;\\n} | render | plainfn.js:1:1:3:1 | functio ... div>;\\n} | | plainfn.js:5:1:7:1 | functio ... iv");\\n} | render | plainfn.js:5:1:7:1 | functio ... iv");\\n} | | plainfn.js:9:1:12:1 | functio ... rn x;\\n} | render | plainfn.js:9:1:12:1 | functio ... rn x;\\n} | @@ -93,6 +94,7 @@ test_ReactComponent_ref | es6.js:14:1:20:1 | class H ... }\\n} | es6.js:16:9:16:12 | this | | es6.js:14:1:20:1 | class H ... }\\n} | es6.js:17:9:17:12 | this | | es6.js:14:1:20:1 | class H ... }\\n} | es6.js:18:9:18:12 | this | +| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | exportedComponent.jsx:1:8:1:7 | this | | namedImport.js:3:1:3:28 | class C ... nent {} | namedImport.js:3:27:3:26 | this | | namedImport.js:5:1:5:20 | class D extends C {} | namedImport.js:5:19:5:18 | this | | plainfn.js:1:1:3:1 | functio ... div>;\\n} | plainfn.js:1:1:1:0 | this | @@ -189,6 +191,7 @@ test_ReactComponent_getADirectPropsSource | es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | es5.js:20:24:20:33 | this.props | | es6.js:1:1:8:1 | class H ... ;\\n }\\n} | es6.js:1:37:1:36 | args | | es6.js:1:1:8:1 | class H ... ;\\n }\\n} | es6.js:3:24:3:33 | this.props | +| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | exportedComponent.jsx:1:29:1:33 | props | | namedImport.js:3:1:3:28 | class C ... nent {} | namedImport.js:3:27:3:26 | args | | namedImport.js:5:1:5:20 | class D extends C {} | namedImport.js:5:19:5:18 | args | | plainfn.js:1:1:3:1 | functio ... div>;\\n} | plainfn.js:1:16:1:20 | props | @@ -208,6 +211,7 @@ test_ReactComponent_getADirectPropsSource | thisAccesses.js:47:1:52:1 | class C ... }\\n} | thisAccesses.js:48:18:48:18 | y | test_ReactComponent_getACandidatePropsValue | es5.js:8:13:8:19 | 'world' | +| importedComponent.jsx:4:32:4:36 | color | | props.js:5:46:5:67 | "propFr ... tProps" | | props.js:7:22:7:34 | "propFromJSX" | | props.js:9:33:9:53 | "propFr ... ructor" | @@ -222,6 +226,7 @@ test_ReactComponent | es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | | es6.js:1:1:8:1 | class H ... ;\\n }\\n} | | es6.js:14:1:20:1 | class H ... }\\n} | +| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | | namedImport.js:3:1:3:28 | class C ... nent {} | | namedImport.js:5:1:5:20 | class D extends C {} | | plainfn.js:1:1:3:1 | functio ... div>;\\n} | @@ -248,6 +253,7 @@ test_ReactComponent_getAPropRead | es5.js:1:31:11:1 | {\\n dis ... ;\\n }\\n} | name | es5.js:4:24:4:38 | this.props.name | | es5.js:18:33:22:1 | {\\n ren ... ;\\n }\\n} | name | es5.js:20:24:20:38 | this.props.name | | es6.js:1:1:8:1 | class H ... ;\\n }\\n} | name | es6.js:3:24:3:38 | this.props.name | +| exportedComponent.jsx:1:8:3:1 | functio ... r}}/>\\n} | color | exportedComponent.jsx:2:32:2:42 | props.color | | plainfn.js:1:1:3:1 | functio ... div>;\\n} | name | plainfn.js:2:22:2:31 | props.name | | preact.js:1:1:7:1 | class H ... }\\n} | name | preact.js:3:9:3:18 | props.name | | probably-a-component.js:1:1:6:1 | class H ... }\\n} | name | probably-a-component.js:3:9:3:23 | this.props.name |