Skip to content

Commit 4a59e69

Browse files
authored
Merge pull request github#4564 from asgerf/js/react-hooks
Approved by esbena
2 parents ecc52a1 + c7667d3 commit 4a59e69

File tree

21 files changed

+668
-65
lines changed

21 files changed

+668
-65
lines changed

change-notes/1.26/analysis-javascript.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
* Angular-specific taint sources and sinks are now recognized by the security queries.
66

7+
* Support for React has improved, with better handling of react hooks, react-router path parameters, lazy-loaded components, and components transformed using `react-redux` and/or `styled-components`.
8+
9+
* Dynamic imports are now analyzed more precisely.
10+
711
* Support for the following frameworks and libraries has been improved:
812
- [@angular/*](https://www.npmjs.com/package/@angular/core)
913
- [AWS Serverless](https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-resource-function.html)
@@ -29,7 +33,13 @@
2933
- [needle](https://www.npmjs.com/package/needle)
3034
- [object-inspect](https://www.npmjs.com/package/object-inspect)
3135
- [pretty-format](https://www.npmjs.com/package/pretty-format)
36+
- [react](https://www.npmjs.com/package/react)
37+
- [react-router-dom](https://www.npmjs.com/package/react-router-dom)
38+
- [react-redux](https://www.npmjs.com/package/react-redux)
39+
- [redis](https://www.npmjs.com/package/redis)
40+
- [redux](https://www.npmjs.com/package/redux)
3241
- [stringify-object](https://www.npmjs.com/package/stringify-object)
42+
- [styled-components](https://www.npmjs.com/package/styled-components)
3343
- [throttle-debounce](https://www.npmjs.com/package/throttle-debounce)
3444
- [underscore](https://www.npmjs.com/package/underscore)
3545

javascript/ql/src/experimental/Security/CWE-94/ServerSideTemplateInjection.ql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ class SSTINunjucksSink extends ServerSideTemplateInjectionSink {
5757
}
5858
}
5959

60+
class LodashTemplateSink extends ServerSideTemplateInjectionSink {
61+
LodashTemplateSink() { this = LodashUnderscore::member("template").getACall().getArgument(0) }
62+
}
63+
6064
from DataFlow::PathNode source, DataFlow::PathNode sink, ServerSideTemplateInjectionConfiguration c
6165
where c.hasFlowPath(source, sink)
6266
select sink.getNode(), source, sink,

javascript/ql/src/meta/analysis-quality/TaintSteps.ql

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,22 @@
1111
import javascript
1212
import CallGraphQuality
1313

14-
class BasicTaintConfiguration extends TaintTracking::Configuration {
15-
BasicTaintConfiguration() { this = "BasicTaintConfiguration" }
16-
}
17-
1814
predicate relevantStep(DataFlow::Node pred, DataFlow::Node succ) {
19-
any(BasicTaintConfiguration cfg).isAdditionalFlowStep(pred, succ) and
15+
(
16+
any(TaintTracking::AdditionalTaintStep dts).step(pred, succ)
17+
or
18+
any(DataFlow::AdditionalFlowStep cfg).step(pred, succ)
19+
or
20+
any(DataFlow::AdditionalFlowStep cfg).step(pred, succ, _, _)
21+
or
22+
any(DataFlow::AdditionalFlowStep cfg).loadStep(pred, succ, _)
23+
or
24+
any(DataFlow::AdditionalFlowStep cfg).storeStep(pred, succ, _)
25+
or
26+
any(DataFlow::AdditionalFlowStep cfg).loadStoreStep(pred, succ, _, _)
27+
or
28+
any(DataFlow::AdditionalFlowStep cfg).loadStoreStep(pred, succ, _)
29+
) and
2030
not pred.getFile() instanceof IgnoredFile and
2131
not succ.getFile() instanceof IgnoredFile
2232
}

javascript/ql/src/semmle/javascript/Promises.qll

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,3 +641,39 @@ private module ClosurePromise {
641641
override predicate step(DataFlow::Node src, DataFlow::Node dst) { src = pred and dst = this }
642642
}
643643
}
644+
645+
private module DynamicImportSteps {
646+
/**
647+
* A step from an export value to its uses via dynamic imports.
648+
*
649+
* For example:
650+
* ```js
651+
* // foo.js
652+
* export default Foo
653+
*
654+
* // bar.js
655+
* let Foo = await import('./foo');
656+
* ```
657+
*/
658+
class DynamicImportStep extends PreCallGraphStep {
659+
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
660+
exists(DynamicImportExpr imprt |
661+
pred = imprt.getImportedModule().getAnExportedValue("default") and
662+
succ = imprt.flow() and
663+
prop = Promises::valueProp()
664+
)
665+
}
666+
667+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
668+
// Special-case code like `(await import(x)).Foo` to boost type tracking depth.
669+
exists(
670+
DynamicImportExpr imprt, string name, DataFlow::Node mid, DataFlow::SourceNode awaited
671+
|
672+
pred = imprt.getImportedModule().getAnExportedValue(name) and
673+
mid.getALocalSource() = imprt.flow() and
674+
PromiseFlow::loadStep(mid, awaited, Promises::valueProp()) and
675+
succ = awaited.getAPropertyRead(name)
676+
)
677+
}
678+
}
679+
}

javascript/ql/src/semmle/javascript/dataflow/Sources.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ module SourceNode {
307307
astNode instanceof FunctionBindExpr or
308308
astNode instanceof DynamicImportExpr or
309309
astNode instanceof ImportSpecifier or
310-
astNode instanceof ImportMetaExpr
310+
astNode instanceof ImportMetaExpr or
311+
astNode instanceof TaggedTemplateExpr
311312
)
312313
or
313314
DataFlow::parameterNode(this, _)

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ module TaintTracking {
327327
* taint to flow from `v` to any read of `c2.state.p`, where `c2`
328328
* also is an instance of `C`.
329329
*/
330-
private class ReactComponentStateTaintStep extends AdditionalTaintStep, DataFlow::ValueNode {
330+
private class ReactComponentStateTaintStep extends AdditionalTaintStep {
331331
DataFlow::Node source;
332332

333333
ReactComponentStateTaintStep() {
@@ -358,7 +358,7 @@ module TaintTracking {
358358
* taint to flow from `v` to any read of `c2.props.p`, where `c2`
359359
* also is an instance of `C`.
360360
*/
361-
private class ReactComponentPropsTaintStep extends AdditionalTaintStep, DataFlow::ValueNode {
361+
private class ReactComponentPropsTaintStep extends AdditionalTaintStep {
362362
DataFlow::Node source;
363363

364364
ReactComponentPropsTaintStep() {

javascript/ql/src/semmle/javascript/frameworks/ComposedFunctions.qll

Lines changed: 115 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,116 @@
55
import javascript
66

77
/**
8-
* A function composed from a collection of functions.
8+
* A call to a function that constructs a function composition `f(g(h(...)))` from a
9+
* series functions `f, g, h, ...`.
910
*/
10-
private class ComposedFunction extends DataFlow::CallNode {
11-
ComposedFunction() {
12-
exists(string name |
13-
name = "just-compose" or
14-
name = "compose-function"
15-
|
16-
this = DataFlow::moduleImport(name).getACall()
17-
)
18-
or
19-
this = LodashUnderscore::member("flow").getACall()
11+
class FunctionCompositionCall extends DataFlow::CallNode {
12+
FunctionCompositionCall::Range range;
13+
14+
FunctionCompositionCall() { this = range }
15+
16+
/**
17+
* Gets the `i`th function in the composition `f(g(h(...)))`, counting from left to right.
18+
*
19+
* Note that this is the opposite of the order in which the function are invoked,
20+
* that is, `g` occurs later than `f` in `f(g(...))` but is invoked before `f`.
21+
*/
22+
DataFlow::Node getOperandNode(int i) { result = range.getOperandNode(i) }
23+
24+
/** Gets a node holding one of the functions to be composed. */
25+
final DataFlow::Node getAnOperandNode() { result = getOperandNode(_) }
26+
27+
/**
28+
* Gets the function flowing into the `i`th function in the composition `f(g(h(...)))`.
29+
*
30+
* Note that this is the opposite of the order in which the function are invoked,
31+
* that is, `g` occurs later than `f` in `f(g(...))` but is invoked before `f`.
32+
*/
33+
final DataFlow::FunctionNode getOperandFunction(int i) {
34+
result = getOperandNode(i).getALocalSource()
2035
}
2136

37+
/** Gets any of the functions being composed. */
38+
final DataFlow::Node getAnOperandFunction() { result = getOperandFunction(_) }
39+
40+
/** Gets the number of functions being composed. */
41+
int getNumOperand() { result = range.getNumOperand() }
42+
}
43+
44+
/**
45+
* Companion module to the `FunctionCompositionCall` class.
46+
*/
47+
module FunctionCompositionCall {
2248
/**
23-
* Gets the ith function in this composition.
49+
* Class that determines the set of values in `FunctionCompositionCall`.
50+
*
51+
* May be subclassed to classify more calls as function compositions.
2452
*/
25-
DataFlow::FunctionNode getFunction(int i) { result.flowsTo(getArgument(i)) }
53+
abstract class Range extends DataFlow::CallNode {
54+
/**
55+
* Gets the function flowing into the `i`th function in the composition `f(g(h(...)))`.
56+
*/
57+
abstract DataFlow::Node getOperandNode(int i);
58+
59+
/** Gets the number of functions being composed. */
60+
abstract int getNumOperand();
61+
}
62+
63+
/**
64+
* A function composition call that accepts its operands in an array or
65+
* via the arguments list.
66+
*
67+
* For simplicity, we model every composition function as if it supported this.
68+
*/
69+
abstract private class WithArrayOverloading extends Range {
70+
/** Gets the `i`th argument to the call or the `i`th array element passed into the call. */
71+
DataFlow::Node getEffectiveArgument(int i) {
72+
result = getArgument(0).(DataFlow::ArrayCreationNode).getElement(i)
73+
or
74+
not getArgument(0) instanceof DataFlow::ArrayCreationNode and
75+
result = getArgument(i)
76+
}
77+
78+
override int getNumOperand() {
79+
result = getArgument(0).(DataFlow::ArrayCreationNode).getSize()
80+
or
81+
not getArgument(0) instanceof DataFlow::ArrayCreationNode and
82+
result = getNumArgument()
83+
}
84+
}
85+
86+
/** A call whose arguments are functions `f,g,h` which are composed into `f(g(h(...))` */
87+
private class RightToLeft extends WithArrayOverloading {
88+
RightToLeft() {
89+
this = DataFlow::moduleImport(["compose-function"]).getACall()
90+
or
91+
this = DataFlow::moduleMember(["redux", "ramda"], "compose").getACall()
92+
or
93+
this = LodashUnderscore::member("flowRight").getACall()
94+
}
95+
96+
override DataFlow::Node getOperandNode(int i) { result = getEffectiveArgument(i) }
97+
}
98+
99+
/** A call whose arguments are functions `f,g,h` which are composed into `f(g(h(...))` */
100+
private class LeftToRight extends WithArrayOverloading {
101+
LeftToRight() {
102+
this = DataFlow::moduleImport("just-compose").getACall()
103+
or
104+
this = LodashUnderscore::member("flow").getACall()
105+
}
106+
107+
override DataFlow::Node getOperandNode(int i) {
108+
result = getEffectiveArgument(getNumOperand() - i - 1)
109+
}
110+
}
26111
}
27112

28113
/**
29114
* A taint step for a composed function.
30115
*/
31116
private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintStep {
32-
ComposedFunction composed;
117+
FunctionCompositionCall composed;
33118
DataFlow::CallNode call;
34119

35120
ComposedFunctionTaintStep() {
@@ -38,25 +123,24 @@ private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintSt
38123
}
39124

40125
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
41-
exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getFunction(fnIndex) |
126+
exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) |
127+
// flow into the first function
128+
fnIndex = composed.getNumOperand() - 1 and
129+
exists(int callArgIndex |
130+
pred = call.getArgument(callArgIndex) and
131+
succ = fn.getParameter(callArgIndex)
132+
)
133+
or
134+
// flow through the composed functions
135+
exists(DataFlow::FunctionNode predFn | predFn = composed.getOperandFunction(fnIndex + 1) |
136+
pred = predFn.getReturnNode() and
137+
succ = fn.getParameter(0)
138+
)
139+
or
42140
// flow out of the composed call
43-
fnIndex = composed.getNumArgument() - 1 and
44-
pred = fn.getAReturn() and
141+
fnIndex = 0 and
142+
pred = fn.getReturnNode() and
45143
succ = this
46-
or
47-
if fnIndex = 0
48-
then
49-
// flow into the first composed function
50-
exists(int callArgIndex |
51-
pred = call.getArgument(callArgIndex) and
52-
succ = fn.getParameter(callArgIndex)
53-
)
54-
else
55-
// flow through the composed functions
56-
exists(DataFlow::FunctionNode predFn | predFn = composed.getFunction(fnIndex - 1) |
57-
pred = predFn.getAReturn() and
58-
succ = fn.getParameter(0)
59-
)
60144
)
61145
}
62146
}

javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,12 @@ module LodashUnderscore {
407407
"shuffle", "sample", "toArray", "partition", "compact", "first", "initial", "last",
408408
"rest", "flatten", "without", "difference", "uniq", "unique", "unzip", "transpose",
409409
"object", "chunk", "values", "mapObject", "pick", "omit", "defaults", "clone", "tap",
410-
"identity"] and
410+
"identity",
411+
// String category
412+
"camelCase", "capitalize", "deburr", "kebabCase", "lowerCase", "lowerFirst", "pad",
413+
"padEnd", "padStart", "repeat", "replace", "snakeCase", "split", "startCase", "toLower",
414+
"toUpper", "trim", "trimEnd", "trimStart", "truncate", "unescape", "upperCase",
415+
"upperFirst", "words"] and
411416
pred = call.getArgument(0) and
412417
succ = call
413418
or

0 commit comments

Comments
 (0)