Skip to content

Commit 307187f

Browse files
am0o0am0o0
committed
V1
1 parent 5259a6e commit 307187f

29 files changed

+1675
-41
lines changed

‎javascript/ql/src/experimental/Security/CWE-340/TokenBuiltFromUUID.qhelp

Lines changed: 0 additions & 40 deletions
This file was deleted.

‎javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/Bombs_jszip.ql

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* @name User-controlled file decompression
3+
* @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.8
7+
* @precision medium
8+
* @id js/user-controlled-file-decompression
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-409
12+
*/
13+
14+
import javascript
15+
import CommandLineSource
16+
17+
class BombConfiguration extends TaintTracking::Configuration {
18+
BombConfiguration() { this = "DecompressionBombs" }
19+
20+
override predicate isSource(DataFlow::Node source) {
21+
source instanceof RemoteFlowSource
22+
or
23+
source = any(CommandLineFlowSource cls).asSource()
24+
or
25+
exists(Function f | source.asExpr() = f.getAParameter() |
26+
not exists(source.getALocalSource().getStringValue())
27+
)
28+
or
29+
exists(FileSystemReadAccess fsra | source = fsra.getADataNode() |
30+
not exists(fsra.getALocalSource().getStringValue())
31+
)
32+
or
33+
exists(Function f | source.asExpr() = f.getAParameter() |
34+
not exists(source.getALocalSource().getStringValue())
35+
)
36+
}
37+
38+
override predicate isSink(DataFlow::Node sink) {
39+
exists(API::Node loadAsync | loadAsync = API::moduleImport("jszip").getMember("loadAsync") |
40+
sink = loadAsync.getParameter(0).asSink() and sanitizer(loadAsync)
41+
)
42+
}
43+
44+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
45+
// additional taint step for fs.readFile(pred)
46+
// It can be global additional step too
47+
exists(DataFlow::CallNode n | n = DataFlow::moduleMember("fs", "readFile").getACall() |
48+
pred = n.getArgument(0) and succ = n.getABoundCallbackParameter(1, 1)
49+
)
50+
}
51+
}
52+
53+
predicate sanitizer(API::Node loadAsync) {
54+
not exists(loadAsync.getASuccessor*().getMember("_data").getMember("uncompressedSize"))
55+
}
56+
57+
from BombConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
58+
where cfg.hasFlowPath(source, sink)
59+
select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(),
60+
"potentially untrusted source"

‎javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/Bombs_node-tar.ql

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* @name User-controlled file decompression
3+
* @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.8
7+
* @precision medium
8+
* @id js/user-controlled-file-decompression
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-409
12+
*/
13+
14+
import javascript
15+
import DataFlow::PathGraph
16+
import API
17+
import semmle.javascript.Concepts
18+
import ReadableAdditionalStep
19+
import CommandLineSource
20+
21+
class BombConfiguration extends TaintTracking::Configuration {
22+
BombConfiguration() { this = "DecompressionBombs" }
23+
24+
override predicate isSource(DataFlow::Node source) {
25+
source instanceof RemoteFlowSource
26+
or
27+
// cli Sources
28+
source = any(CommandLineFlowSource cls).asSource()
29+
or
30+
exists(Function f | source.asExpr() = f.getAParameter() |
31+
not exists(source.getALocalSource().getStringValue())
32+
)
33+
or
34+
exists(FileSystemReadAccess fsra | source = fsra.getADataNode() |
35+
not exists(fsra.getALocalSource().getStringValue())
36+
)
37+
or
38+
source.asExpr() =
39+
API::moduleImport("tar")
40+
.getMember(["x", "extract"])
41+
.getParameter(0)
42+
.asSink()
43+
.asExpr()
44+
.(ObjectExpr)
45+
.getAChild()
46+
.(Property)
47+
.getAChild() and
48+
not source.getALocalSource().mayHaveStringValue(_)
49+
}
50+
51+
override predicate isSink(DataFlow::Node sink) {
52+
exists(API::Node tarExtract |
53+
tarExtract = API::moduleImport("tar").getMember(["x", "extract"])
54+
|
55+
(
56+
// piping tar.x()
57+
sink = tarExtract.getACall()
58+
or
59+
// tar.x({file: filename})
60+
// and we don't have a "maxReadSize: ANum" option
61+
sink.asExpr() =
62+
tarExtract
63+
.getParameter(0)
64+
.asSink()
65+
.asExpr()
66+
.(ObjectExpr)
67+
.getAChild()
68+
.(Property)
69+
.getAChild*() and
70+
tarExtract
71+
.getParameter(0)
72+
.asSink()
73+
.asExpr()
74+
.(ObjectExpr)
75+
.getAChild()
76+
.(Property)
77+
.getAChild*()
78+
.(Label)
79+
.getName() = "file"
80+
) and
81+
nodeTarSanitizer(tarExtract)
82+
)
83+
}
84+
85+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
86+
readablePipeAdditionalTaintStep(pred, succ)
87+
or
88+
exists(FileSystemReadAccess cn | pred = cn.getADataNode() and succ = cn.getAPathArgument())
89+
or
90+
exists(DataFlow::Node sinkhelper, AstNode an |
91+
an = sinkhelper.asExpr().(ObjectExpr).getAChild().(Property).getAChild()
92+
|
93+
pred.asExpr() = an and
94+
succ = sinkhelper
95+
)
96+
or
97+
exists(API::Node n | n = API::moduleImport("tar") |
98+
pred = n.asSource() and
99+
(
100+
succ = n.getMember("x").getACall() or
101+
succ = n.getMember("x").getACall().getArgument(0)
102+
)
103+
)
104+
}
105+
}
106+
107+
predicate nodeTarSanitizer(API::Node tarExtract) {
108+
not tarExtract
109+
.getParameter(0)
110+
.asSink()
111+
.asExpr()
112+
.(ObjectExpr)
113+
.getAChild()
114+
.(Property)
115+
.getAChild*()
116+
.(Label)
117+
.getName() = "maxReadSize"
118+
}
119+
120+
from BombConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
121+
where cfg.hasFlowPath(source, sink)
122+
select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(),
123+
"potentially untrusted source"

‎javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/Bombs_zlib-Pako-AdmZip.ql

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* @name User-controlled file decompression
3+
* @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.8
7+
* @precision medium
8+
* @id js/user-controlled-file-decompression
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-409
12+
*/
13+
14+
import javascript
15+
import DataFlow::PathGraph
16+
import API
17+
import semmle.javascript.security.dataflow.IndirectCommandInjectionCustomizations
18+
import ReadableAdditionalStep
19+
import CommandLineSource
20+
21+
class BombConfiguration extends TaintTracking::Configuration {
22+
BombConfiguration() { this = "DecompressionBombs" }
23+
24+
override predicate isSource(DataFlow::Node source) {
25+
source = any(RemoteFlowSource rfs)
26+
or
27+
// cli Sources
28+
source = any(CommandLineFlowSource cls).asSource()
29+
or
30+
exists(Function f | source.asExpr() = f.getAParameter() |
31+
not exists(source.getALocalSource().getStringValue())
32+
)
33+
or
34+
exists(FileSystemReadAccess fsra | source = fsra.getADataNode() |
35+
not exists(fsra.getALocalSource().getStringValue())
36+
)
37+
or
38+
exists(DataFlow::NewNode nn, DataFlow::Node n | nn = n.(NewNode) |
39+
source = nn.getArgument(0) and
40+
nn.getCalleeName() = "AdmZip" and
41+
not exists(source.getALocalSource().getStringValue())
42+
)
43+
}
44+
45+
override predicate isSink(DataFlow::Node sink) {
46+
// we don't have a "maxOutputLength: ANum" option
47+
exists(API::Node zlib |
48+
zlib =
49+
API::moduleImport("zlib")
50+
.getMember([
51+
"createGunzip", "createBrotliDecompress", "createUnzip", "createInflate",
52+
"createInflateRaw"
53+
]) and
54+
sink = zlib.getACall() and
55+
zlibSanitizer(zlib, 0)
56+
or
57+
zlib =
58+
API::moduleImport("zlib")
59+
.getMember([
60+
"gunzip", "gunzipSync", "unzip", "unzipSync", "brotliDecompress",
61+
"brotliDecompressSync", "inflateSync", "inflateRawSync", "inflate", "inflateRaw"
62+
]) and
63+
sink = zlib.getACall().getArgument(0) and
64+
zlibSanitizer(zlib, 1)
65+
)
66+
or
67+
sink =
68+
[
69+
DataFlow::moduleMember("pako", ["inflate", "inflateRaw", "ungzip"])
70+
.getACall()
71+
.getArgument(0)
72+
]
73+
or
74+
exists(API::Node n | n = API::moduleImport("adm-zip").getInstance() |
75+
(
76+
sink = n.getMember(["extractAllTo", "extractEntryTo", "readAsText"]).getReturn().asSource()
77+
or
78+
sink =
79+
n.getMember("getEntries").getASuccessor*().getMember("getData").getReturn().asSource()
80+
)
81+
)
82+
}
83+
84+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
85+
readablePipeAdditionalTaintStep(pred, succ)
86+
or
87+
// succ = new Uint8Array(pred)
88+
exists(DataFlow::Node n, NewExpr ne | ne = n.asExpr().(NewExpr) |
89+
pred.asExpr() = ne.getArgument(0) and
90+
succ.asExpr() = ne and
91+
ne.getCalleeName() = "Uint8Array"
92+
)
93+
or
94+
// AdmZip
95+
exists(API::Node n | n = API::moduleImport("adm-zip") |
96+
pred = n.getParameter(0).asSink() and
97+
(
98+
succ =
99+
n.getInstance()
100+
.getMember(["extractAllTo", "extractEntryTo", "readAsText"])
101+
.getReturn()
102+
.asSource() or
103+
succ =
104+
n.getInstance()
105+
.getMember("getEntries")
106+
.getASuccessor*()
107+
.getMember("getData")
108+
.getReturn()
109+
.asSource()
110+
)
111+
)
112+
or
113+
// pred.pipe(succ)
114+
// I saw many instances like response.pipe(succ) which I couldn't exactly model this pattern
115+
exists(DataFlow::MethodCallNode n |
116+
n.getMethodName() = "pipe" and
117+
succ = n.getArgument(0) and
118+
pred = n.getReceiver() and
119+
not pred instanceof DataFlow::MethodCallNode
120+
)
121+
}
122+
}
123+
124+
predicate zlibSanitizer(API::Node zlib, int numOfParameter) {
125+
numOfParameter = [0, 1] and
126+
not zlib.getParameter(numOfParameter)
127+
.asSink()
128+
.asExpr()
129+
.(ObjectExpr)
130+
.getAChild()
131+
.(Property)
132+
.getAChild*()
133+
.(Label)
134+
.getName() = "maxOutputLength"
135+
}
136+
137+
from BombConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
138+
where cfg.hasFlowPath(source, sink)
139+
select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(),
140+
"potentially untrusted source"

0 commit comments

Comments
 (0)