4
4
* @kind path-problem
5
5
* @problem.severity error
6
6
* @security-severity 7.8
7
- * @precision medium
8
- * @id js/user-controlled-file -decompression-zlib-pako-admzip
7
+ * @precision high
8
+ * @id js/user-controlled-data -decompression
9
9
* @tags security
10
10
* experimental
11
11
* external/cwe/cwe-409
12
12
*/
13
13
14
14
import javascript
15
- import DataFlow:: PathGraph
16
- import API
17
- import semmle.javascript.security.dataflow.IndirectCommandInjectionCustomizations
18
- import ReadableAdditionalStep
19
15
import CommandLineSource
16
+ import ReadableAdditionalStep
20
17
21
18
class BombConfiguration extends TaintTracking:: Configuration {
22
19
BombConfiguration ( ) { this = "DecompressionBombs" }
23
20
24
21
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
22
exists ( Function f | source .asExpr ( ) = f .getAParameter ( ) |
31
23
not exists ( source .getALocalSource ( ) .getStringValue ( ) )
32
24
)
33
25
or
26
+ source instanceof RemoteFlowSource
27
+ or
28
+ source .asExpr ( ) = any ( Parameter cls )
29
+ or
34
30
exists ( FileSystemReadAccess fsra | source = fsra .getADataNode ( ) |
35
31
not exists ( fsra .getALocalSource ( ) .getStringValue ( ) )
36
32
)
37
33
or
38
- exists ( DataFlow :: NewNode nn , DataFlow :: Node n | nn = n . ( NewNode ) |
39
- source = nn . getArgument ( 0 ) and
40
- nn . getCalleeName ( ) = "AdmZip" and
34
+ exists ( API :: Node node |
35
+ source = node . getParameter ( 0 ) . asSink ( ) and
36
+ node = API :: moduleImport ( "adm-zip" ) and
41
37
not exists ( source .getALocalSource ( ) .getStringValue ( ) )
42
38
)
39
+ or
40
+ exists ( AstNode e |
41
+ e =
42
+ API:: moduleImport ( "tar" )
43
+ .getMember ( [ "x" , "extract" ] )
44
+ .getParameter ( 0 )
45
+ .asSink ( )
46
+ .asExpr ( )
47
+ .( ObjectExpr )
48
+ .getAChild ( )
49
+ .( Property )
50
+ |
51
+ source .asExpr ( ) = e .getAChild ( ) and
52
+ e .getAChild * ( ) .( Label ) .getName ( ) = "file" and
53
+ not source .getALocalSource ( ) .mayHaveStringValue ( _)
54
+ )
43
55
}
44
56
45
57
override predicate isSink ( DataFlow:: Node sink ) {
58
+ // jszip
59
+ exists ( API:: Node loadAsync | loadAsync = API:: moduleImport ( "jszip" ) .getMember ( "loadAsync" ) |
60
+ sink = loadAsync .getParameter ( 0 ) .asSink ( ) and jsZipsanitizer ( loadAsync )
61
+ )
62
+ or
63
+ // node-tar
64
+ exists ( API:: Node tarExtract |
65
+ tarExtract = API:: moduleImport ( "tar" ) .getMember ( [ "x" , "extract" ] )
66
+ |
67
+ (
68
+ // piping tar.x()
69
+ sink = tarExtract .getACall ( )
70
+ or
71
+ // tar.x({file: filename})
72
+ // and we don't have a "maxReadSize: ANum" option
73
+ sink .asExpr ( ) =
74
+ tarExtract
75
+ .getParameter ( 0 )
76
+ .asSink ( )
77
+ .asExpr ( )
78
+ .( ObjectExpr )
79
+ .getAChild ( )
80
+ .( Property )
81
+ .getAChild * ( ) and
82
+ tarExtract
83
+ .getParameter ( 0 )
84
+ .asSink ( )
85
+ .asExpr ( )
86
+ .( ObjectExpr )
87
+ .getAChild ( )
88
+ .( Property )
89
+ .getAChild * ( )
90
+ .( Label )
91
+ .getName ( ) = "file"
92
+ ) and
93
+ nodeTarSanitizer ( tarExtract )
94
+ )
95
+ or
96
+ // zlib
46
97
// we don't have a "maxOutputLength: ANum" option
47
98
exists ( API:: Node zlib |
48
99
zlib =
@@ -64,9 +115,11 @@ class BombConfiguration extends TaintTracking::Configuration {
64
115
zlibSanitizer ( zlib , 1 )
65
116
)
66
117
or
118
+ // pako
67
119
sink =
68
120
DataFlow:: moduleMember ( "pako" , [ "inflate" , "inflateRaw" , "ungzip" ] ) .getACall ( ) .getArgument ( 0 )
69
121
or
122
+ // adm-zip
70
123
exists ( API:: Node n | n = API:: moduleImport ( "adm-zip" ) .getInstance ( ) |
71
124
(
72
125
sink = n .getMember ( [ "extractAllTo" , "extractEntryTo" , "readAsText" ] ) .getReturn ( ) .asSource ( )
@@ -78,8 +131,33 @@ class BombConfiguration extends TaintTracking::Configuration {
78
131
}
79
132
80
133
override predicate isAdditionalTaintStep ( DataFlow:: Node pred , DataFlow:: Node succ ) {
134
+ // additional taint step for fs.readFile(pred)
135
+ // It can be global additional step too
136
+ exists ( DataFlow:: CallNode n | n = DataFlow:: moduleMember ( "fs" , "readFile" ) .getACall ( ) |
137
+ pred = n .getArgument ( 0 ) and succ = n .getABoundCallbackParameter ( 1 , 1 )
138
+ )
139
+ or
140
+ //node-tar
81
141
readablePipeAdditionalTaintStep ( pred , succ )
82
142
or
143
+ exists ( FileSystemReadAccess cn | pred = cn .getADataNode ( ) and succ = cn .getAPathArgument ( ) )
144
+ or
145
+ exists ( DataFlow:: Node sinkhelper , AstNode an |
146
+ an = sinkhelper .asExpr ( ) .( ObjectExpr ) .getAChild ( ) .( Property ) .getAChild ( )
147
+ |
148
+ pred .asExpr ( ) = an and
149
+ succ = sinkhelper
150
+ )
151
+ or
152
+ exists ( API:: Node n | n = API:: moduleImport ( "tar" ) |
153
+ pred = n .asSource ( ) and
154
+ (
155
+ succ = n .getMember ( "x" ) .getACall ( ) or
156
+ succ = n .getMember ( "x" ) .getACall ( ) .getArgument ( 0 )
157
+ )
158
+ )
159
+ or
160
+ // pako
83
161
// succ = new Uint8Array(pred)
84
162
exists ( DataFlow:: Node n , NewExpr ne | ne = n .asExpr ( ) |
85
163
pred .asExpr ( ) = ne .getArgument ( 0 ) and
@@ -117,6 +195,23 @@ class BombConfiguration extends TaintTracking::Configuration {
117
195
}
118
196
}
119
197
198
+ predicate nodeTarSanitizer ( API:: Node tarExtract ) {
199
+ not tarExtract
200
+ .getParameter ( 0 )
201
+ .asSink ( )
202
+ .asExpr ( )
203
+ .( ObjectExpr )
204
+ .getAChild ( )
205
+ .( Property )
206
+ .getAChild * ( )
207
+ .( Label )
208
+ .getName ( ) = "maxReadSize"
209
+ }
210
+
211
+ predicate jsZipsanitizer ( API:: Node loadAsync ) {
212
+ not exists ( loadAsync .getASuccessor * ( ) .getMember ( "_data" ) .getMember ( "uncompressedSize" ) )
213
+ }
214
+
120
215
predicate zlibSanitizer ( API:: Node zlib , int numOfParameter ) {
121
216
numOfParameter = [ 0 , 1 ] and
122
217
not zlib .getParameter ( numOfParameter )
@@ -132,5 +227,5 @@ predicate zlibSanitizer(API::Node zlib, int numOfParameter) {
132
227
133
228
from BombConfiguration cfg , DataFlow:: PathNode source , DataFlow:: PathNode sink
134
229
where cfg .hasFlowPath ( source , sink )
135
- select sink .getNode ( ) , source , sink , "This file extraction depends on a $@." , source .getNode ( ) ,
230
+ select sink .getNode ( ) , source , sink , "This Decompression depends on a $@." , source .getNode ( ) ,
136
231
"potentially untrusted source"
0 commit comments