Some reformatting

python/ql/src/experimental/Security/CWE-022bis/UnsafeUnpackQuery.qll

@@ -61,23 +61,6 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    // Writing the response data to the archive
-    exists(Stdlib::FileLikeObject::InstanceSource is, Node f, MethodCallNode mc |
-      is.flowsTo(f) and
-      mc.calls(f, "write") and
-      nodeFrom = mc.getArg(0) and
-      nodeTo = is.(CallCfgNode).getArg(0)
-    )
-    or
-    // Copying the response data to the archive
-    exists(Stdlib::FileLikeObject::InstanceSource is, Node f, MethodCallNode mc |
-      is.flowsTo(f) and
-      mc = API::moduleImport("shutil").getMember("copyfileobj").getACall() and
-      f = mc.getArg(1) and
-      nodeFrom = mc.getArg(0) and
-      nodeTo = is.(CallCfgNode).getArg(0)
-    )
-    or
     // Reading the response
     exists(MethodCallNode mc |
       nodeFrom = mc.getObject() and
@@ -94,39 +77,22 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
     or
     // Write for access
     exists(MethodCallNode cn |
-      nodeTo = cn.getObject() and
+      nodeFrom = cn.getObject() and
       cn.getMethodName() = "write" and
-      nodeFrom = cn.getArg(0)
+      nodeTo = cn.getArg(0)
     )
     or
     // Retrieve Django uploaded files
     // see HttpRequest.FILES.getlist(): https://docs.djangoproject.com/en/4.1/ref/request-response/#django.http.QueryDict.getlist
     exists(MethodCallNode mc |
-      nodeFrom = mc.getObject() and mc.getMethodName() = ["getlist", "get"] and nodeTo = mc
+      nodeFrom = mc.getObject() and
+      mc.getMethodName() = ["getlist", "get"] and
+      nodeTo = mc
     )
     or
     // Accessing the name or raw content
     exists(AttrRead ar | ar.accesses(nodeFrom, ["name", "raw"]) and ar.flowsTo(nodeTo))
     or
-    //Use of join of filename
-    exists(API::CallNode mcn |
-      mcn = API::moduleImport("os").getMember("path").getMember("join").getACall() and
-      nodeFrom = mcn.getArg(1) and
-      mcn.flowsTo(nodeTo)
-    )
-    or
-    // Read by chunks
-    exists(MethodCallNode mc |
-      nodeFrom = mc.getObject() and mc.getMethodName() = "chunks" and mc.flowsTo(nodeTo)
-    )
-    or
-    // Considering the use of closing()
-    exists(API::CallNode closing |
-      closing = API::moduleImport("contextlib").getMember("closing").getACall() and
-      closing.flowsTo(nodeTo) and
-      nodeFrom = closing.getArg(0)
-    )
-    or
     // Considering the use of "fs"
     exists(API::CallNode fs, MethodCallNode mcn |
       fs =
@@ -141,5 +107,32 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
       nodeFrom = mcn.getArg(0) and
       nodeTo = mcn
     )
+    or
+    //Use of join of filename
+    exists(API::CallNode mcn |
+      mcn = API::moduleImport("os").getMember("path").getMember("join").getACall() and
+      nodeFrom = mcn.getArg(1) and
+      mcn.flowsTo(nodeTo)
+    )
+    or
+    // Read by chunks
+    exists(MethodCallNode mc |
+      nodeFrom = mc.getObject() and mc.getMethodName() = "chunks" and mc.flowsTo(nodeTo)
+    )
+    or
+    // Write access
+    exists(MethodCallNode cn |
+      nodeTo = cn.getObject() and
+      cn.getMethodName() = "write" and
+      nodeFrom = cn.getArg(0)
+    )
+    or
+    // Writing the response data to the archive
+    exists(Stdlib::FileLikeObject::InstanceSource is, Node f, MethodCallNode mc |
+      is.flowsTo(f) and
+      mc.calls(f, "write") and
+      nodeFrom = mc.getArg(0) and
+      nodeTo = is.(CallCfgNode).getArg(0)
+    )
   }
 }

python/ql/test/experimental/query-tests/Security/CWE-022/UnsafeUnpack.py

@@ -8,16 +8,15 @@ app = Flask(__name__)
 # Consider any RemoteFlowSource as a source
 @app.route("/download_from_url")
 def download_from_url():
-    filename = request.args.get('filename', '')
-    if not filename:
-        response = requests.get(filename, stream=True)
-    
-        tarpath = "/tmp/tmp456/tarball.tar.gz"
-        with open(tarpath, "wb") as f:
-              f.write(response.raw.read())
+      filename = request.args.get('filename', '')
+      if not filename:
+            response = requests.get(filename, stream=True)
 
-        untarredpath = "/tmp/tmp123"
-        shutil.unpack_archive(tarpath, untarredpath) # $result=BAD
+            tarpath = "/tmp/tmp456/tarball.tar.gz"
+            with open(tarpath, "wb") as f:
+                  f.write(response.raw.read())
+            untarredpath = "/tmp/tmp123"
+            shutil.unpack_archive(tarpath, untarredpath) # $result=BAD
         
 
 # A source catching an S3 filename download