Python: TarSlip sanitizer: only clear taint on false edge

maybe it was on purpose, will have to investigate FPs when query is good

Author: RasmusWL

python/ql/src/Security/CWE-022/TarSlip.ql

@@ -121,9 +121,11 @@ class ExtractMembersSink extends TaintSink {
 class TarFileInfoSanitizer extends Sanitizer {
     TarFileInfoSanitizer() { this = "TarInfo sanitizer" }
 
+    /** The test `if <path_sanitizing_test>:` clears taint on its `false` edge. */
     override predicate sanitizingEdge(TaintKind taint, PyEdgeRefinement test) {
+        taint instanceof TarFileInfo and
         path_sanitizing_test(test.getTest()) and
-        taint instanceof TarFileInfo
+        test.getSense() = false
     }
 }
 

python/ql/test/query-tests/Security/CWE-022/TarSlip.expected

@@ -15,8 +15,22 @@ edges
 | tarslip.py:34:14:34:16 | tarfile.open | tarslip.py:34:1:34:17 | tarfile.entry |
 | tarslip.py:40:7:40:39 | tarfile.open | tarslip.py:41:24:41:26 | tarfile.open |
 | tarslip.py:40:7:40:39 | tarfile.open | tarslip.py:41:24:41:26 | tarfile.open |
+| tarslip.py:56:7:56:39 | tarfile.open | tarslip.py:57:14:57:16 | tarfile.open |
+| tarslip.py:56:7:56:39 | tarfile.open | tarslip.py:57:14:57:16 | tarfile.open |
+| tarslip.py:57:1:57:17 | tarfile.entry | tarslip.py:59:21:59:25 | tarfile.entry |
+| tarslip.py:57:1:57:17 | tarfile.entry | tarslip.py:59:21:59:25 | tarfile.entry |
+| tarslip.py:57:14:57:16 | tarfile.open | tarslip.py:57:1:57:17 | tarfile.entry |
+| tarslip.py:57:14:57:16 | tarfile.open | tarslip.py:57:1:57:17 | tarfile.entry |
+| tarslip.py:63:7:63:39 | tarfile.open | tarslip.py:64:14:64:16 | tarfile.open |
+| tarslip.py:63:7:63:39 | tarfile.open | tarslip.py:64:14:64:16 | tarfile.open |
+| tarslip.py:64:1:64:17 | tarfile.entry | tarslip.py:68:21:68:25 | tarfile.entry |
+| tarslip.py:64:1:64:17 | tarfile.entry | tarslip.py:68:21:68:25 | tarfile.entry |
+| tarslip.py:64:14:64:16 | tarfile.open | tarslip.py:64:1:64:17 | tarfile.entry |
+| tarslip.py:64:14:64:16 | tarfile.open | tarslip.py:64:1:64:17 | tarfile.entry |
 #select
 | tarslip.py:13:1:13:3 | tar | tarslip.py:12:7:12:39 | tarfile.open | tarslip.py:13:1:13:3 | tarfile.open | Extraction of tarfile from $@ | tarslip.py:12:7:12:39 | Attribute() | a potentially untrusted source |
 | tarslip.py:18:17:18:21 | entry | tarslip.py:16:7:16:39 | tarfile.open | tarslip.py:18:17:18:21 | tarfile.entry | Extraction of tarfile from $@ | tarslip.py:16:7:16:39 | Attribute() | a potentially untrusted source |
 | tarslip.py:37:17:37:21 | entry | tarslip.py:33:7:33:39 | tarfile.open | tarslip.py:37:17:37:21 | tarfile.entry | Extraction of tarfile from $@ | tarslip.py:33:7:33:39 | Attribute() | a potentially untrusted source |
 | tarslip.py:41:24:41:26 | tar | tarslip.py:40:7:40:39 | tarfile.open | tarslip.py:41:24:41:26 | tarfile.open | Extraction of tarfile from $@ | tarslip.py:40:7:40:39 | Attribute() | a potentially untrusted source |
+| tarslip.py:59:21:59:25 | entry | tarslip.py:56:7:56:39 | tarfile.open | tarslip.py:59:21:59:25 | tarfile.entry | Extraction of tarfile from $@ | tarslip.py:56:7:56:39 | Attribute() | a potentially untrusted source |
+| tarslip.py:68:21:68:25 | entry | tarslip.py:63:7:63:39 | tarfile.open | tarslip.py:68:21:68:25 | tarfile.entry | Extraction of tarfile from $@ | tarslip.py:63:7:63:39 | Attribute() | a potentially untrusted source |

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

@@ -56,7 +56,7 @@ def safemembers(members):
 tar = tarfile.open(unsafe_filename_tar)
 for entry in tar:
     if os.path.isabs(entry.name) or ".." in entry.name:
-        tar.extract(entry, "/tmp/unpack/") # TODO: FN
+        tar.extract(entry, "/tmp/unpack/")
 
 
 # OK Sanitized using not
@@ -65,4 +65,4 @@ def safemembers(members):
     # using `if not (os.path.isabs(entry.name) or ".." in entry.name):`
     # would make the sanitizer work, but for the wrong reasons since out library is a bit broken.
     if not os.path.isabs(entry.name):
-        tar.extract(entry, "/tmp/unpack/")
+        tar.extract(entry, "/tmp/unpack/") # TODO: FP