44 * @kind path-problem
55 * @problem.severity error
66 * @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
99 * @tags security
1010 * experimental
1111 * external/cwe/cwe-409
1212 */
1313
1414import javascript
15- import DataFlow:: PathGraph
16- import API
17- import semmle.javascript.security.dataflow.IndirectCommandInjectionCustomizations
18- import ReadableAdditionalStep
1915import CommandLineSource
16+ import ReadableAdditionalStep
2017
2118class BombConfiguration extends TaintTracking:: Configuration {
2219 BombConfiguration ( ) { this = "DecompressionBombs" }
2320
2421 override predicate isSource ( DataFlow:: Node source ) {
25- source = any ( RemoteFlowSource rfs )
26- or
27- // cli Sources
28- source = any ( CommandLineFlowSource cls ) .asSource ( )
29- or
3022 exists ( Function f | source .asExpr ( ) = f .getAParameter ( ) |
3123 not exists ( source .getALocalSource ( ) .getStringValue ( ) )
3224 )
3325 or
26+ source instanceof RemoteFlowSource
27+ or
28+ source .asExpr ( ) = any ( Parameter cls )
29+ or
3430 exists ( FileSystemReadAccess fsra | source = fsra .getADataNode ( ) |
3531 not exists ( fsra .getALocalSource ( ) .getStringValue ( ) )
3632 )
3733 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
4137 not exists ( source .getALocalSource ( ) .getStringValue ( ) )
4238 )
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+ )
4355 }
4456
4557 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
4697 // we don't have a "maxOutputLength: ANum" option
4798 exists ( API:: Node zlib |
4899 zlib =
@@ -64,9 +115,11 @@ class BombConfiguration extends TaintTracking::Configuration {
64115 zlibSanitizer ( zlib , 1 )
65116 )
66117 or
118+ // pako
67119 sink =
68120 DataFlow:: moduleMember ( "pako" , [ "inflate" , "inflateRaw" , "ungzip" ] ) .getACall ( ) .getArgument ( 0 )
69121 or
122+ // adm-zip
70123 exists ( API:: Node n | n = API:: moduleImport ( "adm-zip" ) .getInstance ( ) |
71124 (
72125 sink = n .getMember ( [ "extractAllTo" , "extractEntryTo" , "readAsText" ] ) .getReturn ( ) .asSource ( )
@@ -78,8 +131,33 @@ class BombConfiguration extends TaintTracking::Configuration {
78131 }
79132
80133 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
81141 readablePipeAdditionalTaintStep ( pred , succ )
82142 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
83161 // succ = new Uint8Array(pred)
84162 exists ( DataFlow:: Node n , NewExpr ne | ne = n .asExpr ( ) |
85163 pred .asExpr ( ) = ne .getArgument ( 0 ) and
@@ -117,6 +195,23 @@ class BombConfiguration extends TaintTracking::Configuration {
117195 }
118196}
119197
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+
120215predicate zlibSanitizer ( API:: Node zlib , int numOfParameter ) {
121216 numOfParameter = [ 0 , 1 ] and
122217 not zlib .getParameter ( numOfParameter )
@@ -132,5 +227,5 @@ predicate zlibSanitizer(API::Node zlib, int numOfParameter) {
132227
133228from BombConfiguration cfg , DataFlow:: PathNode source , DataFlow:: PathNode sink
134229where 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 ( ) ,
136231 "potentially untrusted source"
0 commit comments