Skip to content

Commit faaddd4

Browse files
am0o0am0o0
committed
updates for FormParsers and ReadableStream modules, add separate module for Readable Streams, BusBoy RemoteFlowSources is covering more sources now!, modularize
1 parent e81a4fc commit faaddd4

File tree

5 files changed

+662
-247
lines changed

5 files changed

+662
-247
lines changed

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

Lines changed: 119 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,61 +3,141 @@
33
*/
44

55
import javascript
6+
import semmle.javascript.frameworks.ReadableStream
67

78
/**
8-
* A source of remote flow from the `Busboy` library.
9+
* A module for modeling [busboy](https://www.npmjs.com/package/busboy) package
910
*/
10-
private class BusBoyRemoteFlow extends RemoteFlowSource {
11-
BusBoyRemoteFlow() {
12-
this =
13-
API::moduleImport("busboy")
14-
.getInstance()
15-
.getMember("on")
16-
.getParameter(1)
17-
.getAParameter()
18-
.asSource()
11+
module BusBoy {
12+
/**
13+
* A source of remote flow from the `Busboy` library.
14+
*/
15+
private class BusBoyRemoteFlow extends RemoteFlowSource {
16+
BusBoyRemoteFlow() {
17+
exists(API::Node busboyOnEvent |
18+
busboyOnEvent = API::moduleImport("busboy").getReturn().getMember("on")
19+
|
20+
// Files
21+
busboyOnEvent.getParameter(0).asSink().mayHaveStringValue("file") and
22+
// second param of 'file' event is a Readable stream
23+
this = readableStreamDataNode(busboyOnEvent.getParameter(1).getParameter(1)).asSource()
24+
or
25+
// Fields
26+
busboyOnEvent.getParameter(0).asSink().mayHaveStringValue(["file", "field"]) and
27+
this =
28+
API::moduleImport("busboy")
29+
.getReturn()
30+
.getMember("on")
31+
.getParameter(1)
32+
.getAParameter()
33+
.asSource()
34+
)
35+
}
36+
37+
override string getSourceType() { result = "parsed user value from Busbuy" }
1938
}
2039

21-
override string getSourceType() { result = "parsed user value from Busbuy" }
40+
/**
41+
* Holds if busboy file data as additional taint steps according to a Readable Stream type
42+
*
43+
* TODO: I don't know how it can be a global taint step!
44+
*/
45+
predicate busBoyReadableAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
46+
exists(API::Node busboyOnEvent |
47+
busboyOnEvent = API::moduleImport("busboy").getReturn().getMember("on")
48+
|
49+
busboyOnEvent.getParameter(0).asSink().mayHaveStringValue("file") and
50+
customStreamPipeAdditionalTaintStep(busboyOnEvent.getParameter(1).getParameter(1), pred, succ)
51+
)
52+
}
2253
}
2354

2455
/**
25-
* A source of remote flow from the `Formidable` library parsing a HTTP request.
56+
* A module for modeling [formidable](https://www.npmjs.com/package/formidable) package
2657
*/
27-
private class FormidableRemoteFlow extends RemoteFlowSource {
28-
FormidableRemoteFlow() {
29-
exists(API::Node formidable |
30-
formidable = API::moduleImport("formidable").getReturn()
31-
or
32-
formidable = API::moduleImport("formidable").getMember("formidable").getReturn()
33-
or
34-
formidable =
35-
API::moduleImport("formidable").getMember(["IncomingForm", "Formidable"]).getInstance()
36-
|
37-
this =
38-
formidable.getMember("parse").getACall().getABoundCallbackParameter(1, any(int i | i > 0))
39-
)
58+
module Formidable {
59+
/**
60+
* A source of remote flow from the `Formidable` library parsing a HTTP request.
61+
*/
62+
private class FormidableRemoteFlow extends RemoteFlowSource {
63+
FormidableRemoteFlow() {
64+
exists(API::Node formidable |
65+
formidable = API::moduleImport("formidable").getReturn()
66+
or
67+
formidable = API::moduleImport("formidable").getMember("formidable").getReturn()
68+
or
69+
formidable =
70+
API::moduleImport("formidable").getMember(["IncomingForm", "Formidable"]).getInstance()
71+
|
72+
this =
73+
formidable.getMember("parse").getACall().getABoundCallbackParameter(1, any(int i | i > 0))
74+
or
75+
// if callback is not provide a promise will be returned,
76+
// return values contains [fields,files] members
77+
exists(API::Node parseMethod |
78+
parseMethod = formidable.getMember("parse") and parseMethod.getNumParameter() = 1
79+
|
80+
this = parseMethod.getReturn().asSource()
81+
)
82+
or
83+
// event handler
84+
this = formidable.getMember("on").getParameter(1).getAParameter().asSource()
85+
)
86+
}
87+
88+
override string getSourceType() { result = "parsed user value from Formidable" }
4089
}
90+
}
4191

42-
override string getSourceType() { result = "parsed user value from Formidable" }
92+
API::Node test() {
93+
result =
94+
API::moduleImport("multiparty").getMember("Form").getInstance().getMember("on").getASuccessor*()
4395
}
4496

4597
/**
46-
* A source of remote flow from the `Multiparty` library.
98+
* A module for modeling [multiparty](https://www.npmjs.com/package/multiparty) package
4799
*/
48-
private class MultipartyRemoteFlow extends RemoteFlowSource {
49-
MultipartyRemoteFlow() {
50-
exists(API::Node form | form = API::moduleImport("multiparty").getMember("Form").getInstance() |
51-
exists(API::CallNode parse | parse = form.getMember("parse").getACall() |
52-
this = parse.getParameter(1).getAParameter().asSource()
100+
module Multiparty {
101+
/**
102+
* A source of remote flow from the `Multiparty` library.
103+
*/
104+
private class MultipartyRemoteFlow extends RemoteFlowSource {
105+
MultipartyRemoteFlow() {
106+
exists(API::Node form |
107+
form = API::moduleImport("multiparty").getMember("Form").getInstance()
108+
|
109+
exists(API::CallNode parse | parse = form.getMember("parse").getACall() |
110+
this = parse.getParameter(1).getParameter([1, 2]).asSource()
111+
)
112+
or
113+
exists(API::Node on | on = form.getMember("on") |
114+
(
115+
on.getParameter(0).asSink().mayHaveStringValue(["file", "field"]) and
116+
this = on.getParameter(1).getParameter([0, 1]).asSource()
117+
or
118+
on.getParameter(0).asSink().mayHaveStringValue("part") and
119+
this = readableStreamDataNode(on.getParameter(1).getParameter(0)).asSink()
120+
)
121+
)
53122
)
54-
or
55-
exists(API::CallNode on | on = form.getMember("on").getACall() |
56-
on.getArgument(0).mayHaveStringValue(["part", "file", "field"]) and
57-
this = on.getParameter(1).getAParameter().asSource()
58-
)
59-
)
123+
}
124+
125+
override string getSourceType() { result = "parsed user value from Multiparty" }
60126
}
61127

62-
override string getSourceType() { result = "parsed user value from Multiparty" }
128+
/**
129+
* Holds if multiparty part data as additional taint steps according to a Readable Stream type
130+
*
131+
* TODO: I don't know how it can be a global taint step!
132+
*/
133+
predicate multipartyReadableAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
134+
exists(API::Node multipartyOnEvent |
135+
multipartyOnEvent =
136+
API::moduleImport("multiparty").getMember("Form").getInstance().getMember("on")
137+
|
138+
multipartyOnEvent.getParameter(0).asSink().mayHaveStringValue("part") and
139+
customStreamPipeAdditionalTaintStep(multipartyOnEvent.getParameter(1).getParameter(0), pred,
140+
succ)
141+
)
142+
}
63143
}

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

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import javascript
2+
3+
/**
4+
* Holds if there is a step between `fs.createReadStream` and `stream.Readable.from` first parameters to all other piped parameters
5+
*/
6+
predicate readablePipeAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
7+
exists(API::Node receiver |
8+
receiver =
9+
[
10+
API::moduleImport("fs").getMember("createReadStream"),
11+
API::moduleImport("stream").getMember("Readable").getMember("from")
12+
]
13+
|
14+
customStreamPipeAdditionalTaintStep(receiver, pred, succ)
15+
or
16+
pred = receiver.getParameter(0).asSink() and
17+
succ = receiver.getReturn().asSource()
18+
)
19+
}
20+
21+
/**
22+
* additional taint steps for piped stream from `createReadStream` method of `fs/promises.open`
23+
*/
24+
predicate promisesFileHandlePipeAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
25+
exists(API::Node receiver | receiver = nodeJsPromisesFileSystem().getMember("open") |
26+
customStreamPipeAdditionalTaintStep(receiver, pred, succ)
27+
or
28+
pred = receiver.getParameter(0).asSink() and
29+
succ = receiver.getReturn().asSource()
30+
)
31+
}
32+
33+
/**
34+
* Gets nodejs `fs` Promises API
35+
*/
36+
API::Node nodeJsPromisesFileSystem() {
37+
result = [API::moduleImport("fs").getMember("promises"), API::moduleImport("fs/promises")]
38+
}
39+
40+
/**
41+
* Holds if
42+
* or `receiver.pipe(pred).pipe(sth).pipe(succ)`
43+
*
44+
* or `receiver.pipe(sth).pipe(pred).pipe(succ)`
45+
*
46+
* or `receiver.pipe(succ)` and receiver is pred
47+
*
48+
* Receiver can be any method node that support stream pipe method, it can't be a parameter node
49+
*
50+
* Pass receiver method as receiver, not a return value of the receiver method
51+
*/
52+
predicate customStreamPipeAdditionalTaintStep(
53+
API::Node receiver, DataFlow::Node pred, DataFlow::Node succ
54+
) {
55+
// following connect the first pipe parameter to the last pipe parameter
56+
exists(API::Node firstPipe | firstPipe = receiver.getMember("pipe") |
57+
pred = firstPipe.getParameter(0).asSink() and
58+
succ = firstPipe.getASuccessor*().getMember("pipe").getParameter(0).asSink()
59+
)
60+
or
61+
// following connect a pipe parameter to the next pipe parameter
62+
exists(API::Node cn | cn = receiver.getASuccessor+() |
63+
pred = cn.getParameter(0).asSink() and
64+
succ = cn.getReturn().getMember("pipe").getParameter(0).asSink()
65+
)
66+
or
67+
// it is a function that its return value is a Readable stream object
68+
pred = receiver.getReturn().asSource() and
69+
succ = receiver.getReturn().getMember("pipe").getParameter(0).asSink()
70+
or
71+
// it is a Readable stream object
72+
pred = receiver.asSource() and
73+
succ = receiver.getMember("pipe").getParameter(0).asSink()
74+
}
75+
76+
/**
77+
* Holds if
78+
*
79+
* ```js
80+
* await pipeline(
81+
* pred,
82+
* succ_or_pred,
83+
* succ
84+
* )
85+
* ```
86+
*/
87+
predicate streamPipelineAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
88+
// this step connect the a pipeline parameter to the next pipeline parameter
89+
exists(API::CallNode cn, int i |
90+
// we assume that there are maximum 10 pipes mostly or maybe less
91+
i in [0 .. 10] and
92+
cn = nodeJsStream().getMember("pipeline").getACall()
93+
|
94+
pred = cn.getParameter(i).asSink() and
95+
succ = cn.getParameter(i + 1).asSink()
96+
)
97+
or
98+
// this step connect the first pipeline parameter to the next parameters
99+
exists(API::CallNode cn, int i |
100+
// we assume that there are maximum 10 pipes mostly or maybe less
101+
i in [1 .. 10] and
102+
cn = nodeJsStream().getMember("pipeline").getACall()
103+
|
104+
pred = cn.getParameter(0).asSink() and
105+
succ = cn.getParameter(i).asSink()
106+
)
107+
}
108+
109+
/**
110+
* Gets `stream` Promises API
111+
*/
112+
API::Node nodeJsStream() {
113+
result = [API::moduleImport("stream/promises"), API::moduleImport("stream").getMember("promises")]
114+
}
115+
116+
/**
117+
* Gets a Readable Stream method(not a return value of the method)
118+
* and returns all nodes responsible for a data read access
119+
*/
120+
API::Node readableStreamDataNode(API::Node stream) {
121+
result = stream.getMember("read").getReturn()
122+
or
123+
// 'data' event
124+
exists(API::CallNode onEvent | onEvent = stream.getMember("on").getACall() |
125+
result = onEvent.getParameter(1).getParameter(0) and
126+
onEvent.getParameter(0).asSink().mayHaveStringValue("data")
127+
)
128+
or
129+
// 'Readable' event
130+
exists(API::CallNode onEvent | onEvent = stream.getMember("on").getACall() |
131+
result = onEvent.getParameter(1).getReceiver().getMember("read").getReturn() and
132+
onEvent.getParameter(0).asSink().mayHaveStringValue("readable")
133+
)
134+
}

0 commit comments

Comments
 (0)