Some reformatting

Author: Sim4n6

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,20 +77,37 @@ 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
+    // Considering the use of "fs"
+    exists(API::CallNode fs, MethodCallNode mcn |
+      fs =
+        API::moduleImport("django")
+            .getMember("core")
+            .getMember("files")
+            .getMember("storage")
+            .getMember("FileSystemStorage")
+            .getACall() and
+      fs.flowsTo(mcn.getObject()) and
+      mcn.getMethodName() = ["save", "path"] and
+      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
@@ -120,26 +120,19 @@ class UnsafeUnpackingConfig extends TaintTracking::Configuration {
       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)
+    // Write access
+    exists(MethodCallNode cn |
+      nodeTo = cn.getObject() and
+      cn.getMethodName() = "write" and
+      nodeFrom = cn.getArg(0)
     )
     or
-    // Considering the use of "fs"
-    exists(API::CallNode fs, MethodCallNode mcn |
-      fs =
-        API::moduleImport("django")
-            .getMember("core")
-            .getMember("files")
-            .getMember("storage")
-            .getMember("FileSystemStorage")
-            .getACall() and
-      fs.flowsTo(mcn.getObject()) and
-      mcn.getMethodName() = ["save", "path"] and
-      nodeFrom = mcn.getArg(0) and
-      nodeTo = mcn
+    // 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 @@
 # 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())
-
-        untarredpath = "/tmp/tmp123"
-        shutil.unpack_archive(tarpath, untarredpath) # $result=BAD
+      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())
+            untarredpath = "/tmp/tmp123"
+            shutil.unpack_archive(tarpath, untarredpath) # $result=BAD
 
 
 # A source catching an S3 filename download