Merge pull request github#3331 from RasmusWL/python-improve-file-taint

Approved by tausbn

Author: semmle-qlci

python/ql/src/semmle/python/security/strings/External.qll

@@ -183,14 +183,20 @@ private predicate urlparse(ControlFlowNode fromnode, CallNode tonode) {
 
 /** A kind of "taint", representing an open file-like object from an external source. */
 class ExternalFileObject extends TaintKind {
-    ExternalFileObject() { this = "file[" + any(ExternalStringKind key) + "]" }
+    ExternalStringKind valueKind;
+
+    ExternalFileObject() { this = "file[" + valueKind + "]" }
 
     /** Gets the taint kind for the contents of this file */
-    TaintKind getValue() { this = "file[" + result + "]" }
+    TaintKind getValue() { result = valueKind }
 
     override TaintKind getTaintOfMethodResult(string name) {
-        name = "read" and result = this.getValue()
+        name in ["read", "readline"] and result = this.getValue()
+        or
+        name = "readlines" and result.(SequenceKind).getItem() = this.getValue()
     }
+
+    override TaintKind getTaintForIteration() { result = this.getValue() }
 }
 
 /**

python/ql/test/library-tests/taint/strings/Taint.qll

@@ -34,3 +34,11 @@ class ExceptionInfoSource extends TaintSource {
 
     override string toString() { result = "Exception info source" }
 }
+
+class ExternalFileObjectSource extends TaintSource {
+    ExternalFileObjectSource() { this.(NameNode).getId() = "TAINTED_FILE" }
+
+    override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalFileObject }
+
+    override string toString() { result = "Tainted file source" }
+}

python/ql/test/library-tests/taint/strings/TestStep.expected

@@ -132,6 +132,16 @@
 | Taint externally controlled string | test.py:112 | test.py:112:9:112:22 | tainted_string |  |  -->  | Taint externally controlled string | test.py:112 | test.py:112:9:112:30 | Attribute() |  |
 | Taint externally controlled string | test.py:115 | test.py:115:9:115:22 | tainted_string |  |  -->  | Taint externally controlled string | test.py:115 | test.py:115:9:115:30 | Attribute() |  |
 | Taint externally controlled string | test.py:116 | test.py:116:9:116:22 | tainted_string |  |  -->  | Taint externally controlled string | test.py:116 | test.py:116:9:116:33 | Attribute() |  |
+| Taint externally controlled string | test.py:127 | test.py:127:5:127:29 | For |  |  -->  | Taint externally controlled string | test.py:128 | test.py:128:14:128:17 | line |  |
+| Taint file[externally controlled string] | test.py:120 | test.py:120:20:120:31 | TAINTED_FILE |  |  -->  | Taint file[externally controlled string] | test.py:122 | test.py:122:9:122:20 | tainted_file |  |
+| Taint file[externally controlled string] | test.py:120 | test.py:120:20:120:31 | TAINTED_FILE |  |  -->  | Taint file[externally controlled string] | test.py:123 | test.py:123:9:123:20 | tainted_file |  |
+| Taint file[externally controlled string] | test.py:120 | test.py:120:20:120:31 | TAINTED_FILE |  |  -->  | Taint file[externally controlled string] | test.py:124 | test.py:124:9:124:20 | tainted_file |  |
+| Taint file[externally controlled string] | test.py:120 | test.py:120:20:120:31 | TAINTED_FILE |  |  -->  | Taint file[externally controlled string] | test.py:125 | test.py:125:9:125:20 | tainted_file |  |
+| Taint file[externally controlled string] | test.py:120 | test.py:120:20:120:31 | TAINTED_FILE |  |  -->  | Taint file[externally controlled string] | test.py:127 | test.py:127:17:127:28 | tainted_file |  |
+| Taint file[externally controlled string] | test.py:123 | test.py:123:9:123:20 | tainted_file |  |  -->  | Taint externally controlled string | test.py:123 | test.py:123:9:123:27 | Attribute() |  |
+| Taint file[externally controlled string] | test.py:124 | test.py:124:9:124:20 | tainted_file |  |  -->  | Taint externally controlled string | test.py:124 | test.py:124:9:124:31 | Attribute() |  |
+| Taint file[externally controlled string] | test.py:125 | test.py:125:9:125:20 | tainted_file |  |  -->  | Taint [externally controlled string] | test.py:125 | test.py:125:9:125:32 | Attribute() |  |
+| Taint file[externally controlled string] | test.py:127 | test.py:127:17:127:28 | tainted_file |  |  -->  | Taint externally controlled string | test.py:127 | test.py:127:5:127:29 | For |  |
 | Taint json[externally controlled string] | test.py:6 | test.py:6:20:6:45 | Attribute() |  |  -->  | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json |  |
 | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json |  |  -->  | Taint externally controlled string | test.py:7 | test.py:7:9:7:25 | Subscript |  |
 | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:20 | tainted_json |  |  -->  | Taint json[externally controlled string] | test.py:7 | test.py:7:9:7:25 | Subscript |  |

python/ql/test/library-tests/taint/strings/TestTaint.expected

@@ -54,3 +54,8 @@
 | test.py:112 | test_str_methods | Attribute() | externally controlled string |
 | test.py:115 | test_str_methods | Attribute() | externally controlled string |
 | test.py:116 | test_str_methods | Attribute() | externally controlled string |
+| test.py:122 | test_tainted_file | tainted_file | file[externally controlled string] |
+| test.py:123 | test_tainted_file | Attribute() | externally controlled string |
+| test.py:124 | test_tainted_file | Attribute() | externally controlled string |
+| test.py:125 | test_tainted_file | Attribute() | [externally controlled string] |
+| test.py:128 | test_tainted_file | line | externally controlled string |

python/ql/test/library-tests/taint/strings/test.py

@@ -115,3 +115,14 @@ def test_str_methods():
         tainted_string.upper(),
         tainted_string.zfill(100),
     )
+
+def test_tainted_file():
+    tainted_file = TAINTED_FILE
+    test(
+        tainted_file,
+        tainted_file.read(),
+        tainted_file.readline(),
+        tainted_file.readlines(),
+    )
+    for line in tainted_file:
+        test(line)