Merge pull request #11591 from jketema/getenv

C++: Model `secure_getenv` and `_wgetenv` as local flow sources

Author: jketema

cpp/ql/lib/change-notes/2022-12-08-support-getenv-variants.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `secure_getenv` and `_wgetenv` functions are now recognized as local flow sources.

cpp/ql/lib/semmle/code/cpp/models/implementations/Getenv.qll

@@ -1,15 +1,19 @@
 /**
- * Provides an implementation class modeling the POSIX function `getenv`.
+ * Provides an implementation class modeling the POSIX function `getenv` and
+ * various similar functions.
  */
 
 import cpp
 import semmle.code.cpp.models.interfaces.FlowSource
 
 /**
- * The POSIX function `getenv`.
+ * The POSIX function `getenv`, the GNU function `secure_getenv`, and the
+ * Windows function `_wgetenv`.
  */
 class Getenv extends LocalFlowSourceFunction {
-  Getenv() { this.hasGlobalOrStdOrBslName("getenv") }
+  Getenv() {
+    this.hasGlobalOrStdOrBslName("getenv") or this.hasGlobalName(["secure_getenv", "_wgetenv"])
+  }
 
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {
     (

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_sinks_only/defaulttainttracking.cpp

@@ -1,18 +1,6 @@
 #include "../shared.h"
 
-
-
-
-
-
-
-
-
-
 int main() {
-
-
-
   sink(_strdup(getenv("VAR"))); // $ ir MISSING: ast
   sink(strdup(getenv("VAR"))); // $ ast,ir
   sink(unmodeled_function(getenv("VAR"))); // clean by assumption
@@ -59,9 +47,6 @@ void test_outparams() {
     sink(p2); // $ ir MISSING: ast
 }
 
-
-
-
 struct XY {
   int x;
   int y;
@@ -230,24 +215,17 @@ void test_recv() {
 
 // --- send and related functions ---
 
-int send(int, const void*, int, int);
-
-void test_send(char* buffer, int length) {
-  send(0, buffer, length, 0); // $ remote
-}
-
 struct iovec {
   void  *iov_base;
   unsigned iov_len;
 };
 
 int readv(int, const struct iovec*, int);
-int writev(int, const struct iovec*, int);
 
 void sink(const iovec* iovs);
 void sink(iovec);
 
-int test_readv_and_writev(iovec* iovs) {
+void test_readv_and_writev(iovec* iovs) {
   readv(0, iovs, 16);
   sink(iovs); // $ast,ir
   sink(iovs[0]); // $ast,ir
@@ -256,6 +234,4 @@ int test_readv_and_writev(iovec* iovs) {
   char* p = (char*)iovs[1].iov_base;
   sink(p); // $ MISSING: ast,ir
   sink(*p); // $ MISSING: ast,ir
-
-  writev(0, iovs, 16); // $ remote
 }

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_sinks_only/remote-flow-sink.ql

@@ -0,0 +1,20 @@
+/** This tests that we are able to detect local flow sources. */
+
+import cpp
+import TestUtilities.InlineExpectationsTest
+import semmle.code.cpp.security.FlowSources
+
+class LocalFlowSourceTest extends InlineExpectationsTest {
+  LocalFlowSourceTest() { this = "LocalFlowSourceTest" }
+
+  override string getARelevantTag() { result = "local_source" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "local_source" and
+    value = "" and
+    exists(LocalFlowSource node |
+      location = node.getLocation() and
+      element = node.toString()
+    )
+  }
+}

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_sinks_only/remote-flow-sink.expected renamed to cpp/ql/test/library-tests/dataflow/source-sink-tests/local-flow.expected

@@ -0,0 +1,35 @@
+/** This tests that we are able to detect remote flow sources and sinks. */
+
+import cpp
+import TestUtilities.InlineExpectationsTest
+import semmle.code.cpp.security.FlowSources
+
+class RemoteFlowSourceTest extends InlineExpectationsTest {
+  RemoteFlowSourceTest() { this = "RemoteFlowSourceTest" }
+
+  override string getARelevantTag() { result = "remote_source" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "remote_source" and
+    value = "" and
+    exists(RemoteFlowSource node |
+      location = node.getLocation() and
+      element = node.toString()
+    )
+  }
+}
+
+class RemoteFlowSinkTest extends InlineExpectationsTest {
+  RemoteFlowSinkTest() { this = "RemoteFlowSinkTest" }
+
+  override string getARelevantTag() { result = "remote_sink" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "remote_sink" and
+    value = "" and
+    exists(RemoteFlowSink node |
+      location = node.getLocation() and
+      element = node.toString()
+    )
+  }
+}

cpp/ql/test/library-tests/dataflow/source-sink-tests/local-flow.ql

@@ -0,0 +1,28 @@
+char *getenv(const char *name);
+char *secure_getenv(const char *name);
+wchar_t *_wgetenv(const wchar_t *name);
+
+void test_getenv() {
+    void *var1 = getenv("VAR"); // $ local_source
+    void *var2 = secure_getenv("VAR"); // $ local_source
+    void *var3 = _wgetenv(L"VAR"); // $ local_source
+}
+
+int send(int, const void*, int, int);
+
+void test_send(char* buffer, int length) {
+  send(0, buffer, length, 0); // $ remote_sink
+}
+
+struct iovec {
+  void  *iov_base;
+  unsigned iov_len;
+};
+
+int readv(int, const struct iovec*, int);
+int writev(int, const struct iovec*, int);
+
+void test_readv_and_writev(iovec* iovs) {
+  readv(0, iovs, 16); // $ remote_source
+  writev(0, iovs, 16); // $ remote_sink
+}