Merge pull request github#17715 from MathiasVP/fopen-taint

C++: Add taint through `fopen`
jessejay-ch · Oct 10, 2024 · e7da53d · e7da53d
2 parents 3ef49f3 + acac3a0
commit e7da53d
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 1 deletion.
diff --git a/cpp/ql/lib/change-notes/2024-10-09-fopen-taint.md b/cpp/ql/lib/change-notes/2024-10-09-fopen-taint.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added taint flow model for `fopen` and related functions.
diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Fopen.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Fopen.qll
@@ -7,7 +7,7 @@ import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
 /** The function `fopen` and friends. */
-private class Fopen extends Function, AliasFunction, SideEffectFunction {
+private class Fopen extends Function, AliasFunction, SideEffectFunction, TaintFunction {
   Fopen() {
     this.hasGlobalOrStdName(["fopen", "fopen_s", "freopen"])
     or
@@ -47,4 +47,22 @@ private class Fopen extends Function, AliasFunction, SideEffectFunction {
     i = 0 and
     buffer = true
   }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    (
+      this.hasGlobalOrStdName(["fopen", "freopen"]) or
+      this.hasGlobalName(["_wfopen", "_fsopen", "_wfsopen"])
+    ) and
+    input.isParameterDeref(0) and
+    output.isReturnValueDeref()
+    or
+    // The out parameter is a pointer to a `FILE*`.
+    this.hasGlobalOrStdName("fopen_s") and
+    input.isParameterDeref(1) and
+    output.isParameterDeref(0, 2)
+    or
+    this.hasGlobalName(["_open", "_wopen"]) and
+    input.isParameterDeref(0) and
+    output.isReturnValue()
+  }
 }
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -6584,6 +6584,16 @@ WARNING: module 'TaintTracking' has been deprecated and may be removed in future
 | taint.cpp:767:21:767:24 | ref arg path | taint.cpp:768:8:768:11 | path |  |
 | taint.cpp:768:8:768:11 | path | taint.cpp:768:7:768:11 | * ... |  |
 | taint.cpp:778:37:778:42 | call to source | taint.cpp:779:7:779:9 | obj |  |
+| taint.cpp:785:23:785:28 | source | taint.cpp:785:23:785:28 | source |  |
+| taint.cpp:785:23:785:28 | source | taint.cpp:786:18:786:23 | source |  |
+| taint.cpp:785:23:785:28 | source | taint.cpp:790:15:790:20 | source |  |
+| taint.cpp:786:12:786:16 | call to fopen | taint.cpp:787:7:787:7 | f |  |
+| taint.cpp:786:18:786:23 | source | taint.cpp:786:12:786:16 | call to fopen | TAINT |
+| taint.cpp:789:8:789:9 | f2 | taint.cpp:790:11:790:12 | f2 |  |
+| taint.cpp:789:8:789:9 | f2 | taint.cpp:791:7:791:8 | f2 |  |
+| taint.cpp:790:10:790:12 | ref arg & ... | taint.cpp:790:11:790:12 | f2 [inner post update] |  |
+| taint.cpp:790:10:790:12 | ref arg & ... | taint.cpp:791:7:791:8 | f2 |  |
+| taint.cpp:790:11:790:12 | f2 | taint.cpp:790:10:790:12 | & ... |  |
 | vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 |  |
 | vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 |  |
 | vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v |  |

diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
@@ -777,4 +777,16 @@ TaintInheritingContentObject source(bool);
 void test_TaintInheritingContent() {
 	TaintInheritingContentObject obj = source(true);
 	sink(obj.flowFromObject); // $ ir MISSING: ast
+}
+
+FILE* fopen(const char*, const char*);
+int fopen_s(FILE** pFile, const char *filename, const char *mode);
+
+void fopen_test(char* source) {
+	FILE* f = fopen(source, "r");
+	sink(f); // $ ast,ir
+
+	FILE* f2;
+	fopen_s(&f2, source, "r");
+	sink(f2); // $ ast,ir
 }