Skip to content

Commit eb13ab5

Browse files
author
Esben Sparre Andreasen
committed
JS: sharpen js/prototype-pollution with version analysis
1 parent c143e31 commit eb13ab5

File tree

3 files changed

+76
-26
lines changed

3 files changed

+76
-26
lines changed

‎javascript/ql/src/Security/CWE-400/PrototypePollution.ql‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,15 @@
1414
import javascript
1515
import semmle.javascript.security.dataflow.PrototypePollution::PrototypePollution
1616
import DataFlow::PathGraph
17+
import semmle.javascript.dependencies.Dependencies
1718

18-
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
19-
where cfg.hasFlowPath(source, sink)
20-
select sink.getNode(), source, sink, "Prototype pollution caused by merging a user-controlled value from $@.", source, "here"
19+
from
20+
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Dependency dependency,
21+
string dependencyId
22+
where
23+
cfg.hasFlowPath(source, sink) and
24+
dependency = sink.getNode().(Sink).getDependency() and
25+
dependency.info(dependencyId, _)
26+
select sink.getNode(), source, sink,
27+
"Prototype pollution caused by merging a user-controlled value from $@ using a vulnerable version of $@.",
28+
source, "here", dependency, dependencyId

‎javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollution.qll‎

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
import javascript
77
import semmle.javascript.security.TaintedObject
8+
import semmle.javascript.dependencies.Dependencies
9+
import semmle.javascript.dependencies.SemVer
810

911
module PrototypePollution {
1012
/**
@@ -47,6 +49,11 @@ module PrototypePollution {
4749
* Gets the type of data that can taint this sink.
4850
*/
4951
abstract DataFlow::FlowLabel getAFlowLabel();
52+
53+
/**
54+
* Gets the dependency that defines this sink.
55+
*/
56+
abstract Dependency getDependency();
5057
}
5158

5259
/**
@@ -103,36 +110,73 @@ module PrototypePollution {
103110

104111
override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() }
105112
}
106-
107-
string getModuleName(ExtendCall call) {
108-
call = DataFlow::moduleImport(result).getACall() or
109-
call = DataFlow::moduleMember(result, _).getACall()
110-
}
111113

112114
class DeepExtendSink extends Sink {
113115
ExtendCall call;
114116

117+
Dependency dependency;
118+
115119
DeepExtendSink() {
116120
this = call.getASourceOperand() and
117-
call.isDeep() and
118-
exists(string moduleName | moduleName = getModuleName(call) |
119-
moduleName = "lodash" + any(string s) or
120-
moduleName = "just-extend" or
121-
moduleName = "extend" or
122-
moduleName = "extend2" or
123-
moduleName = "node.extend" or
124-
moduleName = "merge" or
125-
moduleName = "smart-extend" or
126-
moduleName = "js-extend" or
127-
moduleName = "deep" or
128-
moduleName = "defaults-deep"
129-
)
121+
isVulnerableDeepExtendCall(call, dependency)
130122
}
131123

132124
override DataFlow::FlowLabel getAFlowLabel() {
133125
result = TaintedObject::label()
134126
or
135127
result = TaintedObjectWrapper::label()
136128
}
129+
130+
override Dependency getDependency() { result = dependency }
131+
}
132+
133+
/**
134+
* Holds if `call` is vulnerable to prototype pollution because the callee is defined by `dep`.
135+
*/
136+
predicate isVulnerableDeepExtendCall(ExtendCall call, Dependency dep) {
137+
call.isDeep() and
138+
(
139+
call = DataFlow::dependencyModuleImport(dep).getAMemberCall(_) or
140+
call = DataFlow::dependencyModuleImport(dep).getACall()
141+
) and
142+
exists(DependencySemVer version, string id | dep.info(id, version) |
143+
id = "assign-deep" and
144+
version.maybeBefore("0.4.7")
145+
or
146+
id = "deep"
147+
or
148+
id = "deep-extend" and
149+
version.maybeBefore("0.5.1")
150+
or
151+
id = "defaults-deep" and
152+
version.maybeBefore("0.2.4")
153+
or
154+
id = "extend" and
155+
(version.maybeBefore("2.0.2") or version.maybeBetween("3.0.0", "3.0.2"))
156+
or
157+
id = "extend2"
158+
or
159+
id = "js-extend"
160+
or
161+
id = "just-extend" and
162+
version.maybeBefore("4.0.1")
163+
or
164+
id = "lodash" + any(string s) and
165+
version.maybeBefore("4.17.11")
166+
or
167+
id = "merge" and
168+
version.maybeBefore("1.2.1")
169+
or
170+
id = "merge-deep" and
171+
version.maybeBefore("3.0.1")
172+
or
173+
id = "merge-options" and
174+
version.maybeBefore("1.0.1")
175+
or
176+
id = "node.extend" and
177+
(version.maybeBefore("1.1.7") or version.maybeBetween("2.0.0", "2.0.1"))
178+
or
179+
id = "smart-extend"
180+
)
137181
}
138182
}

‎javascript/ql/test/query-tests/Security/CWE-400/PrototypePollution.expected‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
nodes
2-
| src-non-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo |
32
| src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo |
43
| src-vulnerable-lodash/tst.js:10:17:12:5 | {\\n ... K\\n } |
54
| src-vulnerable-lodash/tst.js:11:16:11:30 | req.query.value |
@@ -11,7 +10,6 @@ edges
1110
| src-vulnerable-lodash/tst.js:15:14:15:28 | req.query.value | src-vulnerable-lodash/tst.js:18:16:18:25 | opts.thing |
1211
| src-vulnerable-lodash/tst.js:18:16:18:25 | opts.thing | src-vulnerable-lodash/tst.js:17:17:19:5 | {\\n ... K\\n } |
1312
#select
14-
| src-non-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-non-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-non-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | Prototype pollution caused by merging a user-controlled value from $@. | src-non-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | here |
15-
| src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | Prototype pollution caused by merging a user-controlled value from $@. | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | here |
16-
| src-vulnerable-lodash/tst.js:10:17:12:5 | {\\n ... K\\n } | src-vulnerable-lodash/tst.js:11:16:11:30 | req.query.value | src-vulnerable-lodash/tst.js:10:17:12:5 | {\\n ... K\\n } | Prototype pollution caused by merging a user-controlled value from $@. | src-vulnerable-lodash/tst.js:11:16:11:30 | req.query.value | here |
17-
| src-vulnerable-lodash/tst.js:17:17:19:5 | {\\n ... K\\n } | src-vulnerable-lodash/tst.js:15:14:15:28 | req.query.value | src-vulnerable-lodash/tst.js:17:17:19:5 | {\\n ... K\\n } | Prototype pollution caused by merging a user-controlled value from $@. | src-vulnerable-lodash/tst.js:15:14:15:28 | req.query.value | here |
13+
| src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | Prototype pollution caused by merging a user-controlled value from $@ using a vulnerable version of $@. | src-vulnerable-lodash/tst.js:7:17:7:29 | req.query.foo | here | src-vulnerable-lodash/package.json:3:19:3:26 | "4.17.4" | lodash |
14+
| src-vulnerable-lodash/tst.js:10:17:12:5 | {\\n ... K\\n } | src-vulnerable-lodash/tst.js:11:16:11:30 | req.query.value | src-vulnerable-lodash/tst.js:10:17:12:5 | {\\n ... K\\n } | Prototype pollution caused by merging a user-controlled value from $@ using a vulnerable version of $@. | src-vulnerable-lodash/tst.js:11:16:11:30 | req.query.value | here | src-vulnerable-lodash/package.json:3:19:3:26 | "4.17.4" | lodash |
15+
| src-vulnerable-lodash/tst.js:17:17:19:5 | {\\n ... K\\n } | src-vulnerable-lodash/tst.js:15:14:15:28 | req.query.value | src-vulnerable-lodash/tst.js:17:17:19:5 | {\\n ... K\\n } | Prototype pollution caused by merging a user-controlled value from $@ using a vulnerable version of $@. | src-vulnerable-lodash/tst.js:15:14:15:28 | req.query.value | here | src-vulnerable-lodash/package.json:3:19:3:26 | "4.17.4" | lodash |

0 commit comments

Comments
 (0)