From 74e05c111e462ca5d2eb0b99a2e398cf59f8abd3 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh@semmle.com>
Date: Tue, 19 May 2020 11:07:12 -0700
Subject: [PATCH 01/19] C++: add local flow sources

---
 cpp/ql/src/semmle/code/cpp/models/Models.qll  |  1 +
 .../cpp/models/implementations/Getenv.qll     | 22 +++++++
 .../code/cpp/models/interfaces/FlowSource.qll | 12 +++-
 .../semmle/code/cpp/security/FlowSources.qll  | 66 +++++++++++++++++--
 4 files changed, 95 insertions(+), 6 deletions(-)
 create mode 100644 cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll

diff --git a/cpp/ql/src/semmle/code/cpp/models/Models.qll b/cpp/ql/src/semmle/code/cpp/models/Models.qll
index 75e81fdc318e..f2612d69b844 100644
--- a/cpp/ql/src/semmle/code/cpp/models/Models.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/Models.qll
@@ -1,6 +1,7 @@
 private import implementations.Allocation
 private import implementations.Deallocation
 private import implementations.Fread
+private import implementations.Getenv
 private import implementations.Gets
 private import implementations.IdentityFunction
 private import implementations.Inet
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
new file mode 100644
index 000000000000..764a7dab5dca
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
@@ -0,0 +1,22 @@
+/**
+ * Provides an implementation class modelling the POSIX function `getenv`.
+ */
+import cpp
+import semmle.code.cpp.models.interfaces.FlowSource
+
+/**
+ * The POSIX function `getenv`.
+ */
+class Getenv extends LocalFlowFunction {
+  Getenv() {
+    this.hasGlobalName("getenv")
+  }
+
+  override predicate hasLocalFlowSource (FunctionOutput output, string description) {
+    (
+      output.isReturnValueDeref() or
+      output.isReturnValue()
+    ) and
+    description = "an environment variable"
+  }
+}
\ No newline at end of file
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
index 2c9effaff7cb..8952d7499428 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
@@ -11,7 +11,7 @@ import FunctionInputsAndOutputs
 import semmle.code.cpp.models.Models
 
 /**
- * A library function which returns data read from a network connection.
+ * A library function which returns data that may be read from a network connection.
  */
 abstract class RemoteFlowFunction extends Function {
   /**
@@ -19,3 +19,13 @@ abstract class RemoteFlowFunction extends Function {
    */
   abstract predicate hasRemoteFlowSource(FunctionOutput output, string description);
 }
+
+/**
+ * A library function which returns data that is directly controlled by a user.
+ */
+abstract class LocalFlowFunction extends Function {
+  /**
+   * Holds if data described by `description` flows from `output` of a call to this function.
+   */
+  abstract predicate hasLocalFlowSource(FunctionOutput output, string description);
+}
\ No newline at end of file
diff --git a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
index eff40572c025..62ef81175e5a 100644
--- a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
@@ -13,25 +13,35 @@ abstract class RemoteFlowSource extends DataFlow::Node {
   abstract string getSourceType();
 }
 
-private class TaintedReturnSource extends RemoteFlowSource {
+/** A data flow source of local user input. */
+abstract class LocalFlowSource extends DataFlow::Node {
+  /** Gets a string that describes the type of this local flow source. */
+  abstract string getSourceType();
+}
+
+private class RemoteReturnSource extends RemoteFlowSource {
   string sourceType;
 
-  TaintedReturnSource() {
+  RemoteReturnSource() {
     exists(RemoteFlowFunction func, CallInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getStaticCallTarget() = func and
       func.hasRemoteFlowSource(output, sourceType) and
-      output.isReturnValue()
+      (
+        output.isReturnValue()
+        or
+        output.isReturnValueDeref()
+      )
     )
   }
 
   override string getSourceType() { result = sourceType }
 }
 
-private class TaintedParameterSource extends RemoteFlowSource {
+private class RemoteParameterSource extends RemoteFlowSource {
   string sourceType;
 
-  TaintedParameterSource() {
+  RemoteParameterSource() {
     exists(RemoteFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
@@ -42,3 +52,49 @@ private class TaintedParameterSource extends RemoteFlowSource {
 
   override string getSourceType() { result = sourceType }
 }
+
+private class LocalReturnSource extends LocalFlowSource {
+  string sourceType;
+
+  LocalReturnSource() {
+    exists(LocalFlowFunction func, CallInstruction instr, FunctionOutput output |
+      asInstruction() = instr and
+      instr.getStaticCallTarget() = func and
+      func.hasLocalFlowSource(output, sourceType) and
+      (
+        output.isReturnValue()
+        or
+        output.isReturnValueDeref()
+      )
+    )
+  }
+
+  override string getSourceType() { result = sourceType }
+}
+
+private class LocalParameterSource extends LocalFlowSource {
+  string sourceType;
+
+  LocalParameterSource() {
+    exists(LocalFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
+      asInstruction() = instr and
+      instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
+      func.hasLocalFlowSource(output, sourceType) and
+      output.isParameterDeref(instr.getIndex())
+    )
+  }
+
+  override string getSourceType() { result = sourceType }
+}
+
+private class ArgvSource extends LocalFlowSource {
+  ArgvSource() {
+    exists(Parameter argv |
+      argv.hasName("argv") and
+      argv.getFunction().hasGlobalName("main") and
+      this.asExpr() = argv.getAnAccess()
+    )
+  }
+
+  override string getSourceType() { result = "a command line argument" }
+}

From 31d3e94cec4216465c8644cdf9010014e5835927 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@gmail.com>
Date: Mon, 10 Aug 2020 16:42:44 -0400
Subject: [PATCH 02/19] C++: Grammar/style fixes from code review

Co-authored-by: Jonas Jensen <jbj@github.com>
---
 cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll | 6 +++---
 cpp/ql/src/semmle/code/cpp/security/FlowSources.qll         | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
index 8952d7499428..c0c95b387567 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll
@@ -11,7 +11,7 @@ import FunctionInputsAndOutputs
 import semmle.code.cpp.models.Models
 
 /**
- * A library function which returns data that may be read from a network connection.
+ * A library function that returns data that may be read from a network connection.
  */
 abstract class RemoteFlowFunction extends Function {
   /**
@@ -21,11 +21,11 @@ abstract class RemoteFlowFunction extends Function {
 }
 
 /**
- * A library function which returns data that is directly controlled by a user.
+ * A library function that returns data that is directly controlled by a user.
  */
 abstract class LocalFlowFunction extends Function {
   /**
    * Holds if data described by `description` flows from `output` of a call to this function.
    */
   abstract predicate hasLocalFlowSource(FunctionOutput output, string description);
-}
\ No newline at end of file
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
index 62ef81175e5a..b83fcf720682 100644
--- a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
@@ -96,5 +96,5 @@ private class ArgvSource extends LocalFlowSource {
     )
   }
 
-  override string getSourceType() { result = "a command line argument" }
+  override string getSourceType() { result = "a command-line argument" }
 }

From a94826dc8109d0ad1f9e9b31ce69f3d6aba2cb7d Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh@semmle.com>
Date: Mon, 10 Aug 2020 14:40:05 -0700
Subject: [PATCH 03/19] C++: common superclass for Remote/LocalFlowSource

---
 .../src/semmle/code/cpp/security/FlowSources.qll   | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
index b83fcf720682..4fe5d1bb8736 100644
--- a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
@@ -7,17 +7,17 @@ import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.IR
 import semmle.code.cpp.models.interfaces.FlowSource
 
-/** A data flow source of remote user input. */
-abstract class RemoteFlowSource extends DataFlow::Node {
-  /** Gets a string that describes the type of this remote flow source. */
+/** A data flow source of user input, whether local or remote. */
+abstract class FlowSource extends DataFlow::Node {
+  /** Gets a string that describes the type of this flow source. */
   abstract string getSourceType();
 }
 
+/** A data flow source of remote user input. */
+abstract class RemoteFlowSource extends FlowSource { }
+
 /** A data flow source of local user input. */
-abstract class LocalFlowSource extends DataFlow::Node {
-  /** Gets a string that describes the type of this local flow source. */
-  abstract string getSourceType();
-}
+abstract class LocalFlowSource extends FlowSource { }
 
 private class RemoteReturnSource extends RemoteFlowSource {
   string sourceType;

From 057bb14eeec4c06643cb18c6e14105714e9fbd1e Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Mon, 16 Nov 2020 22:36:05 +0100
Subject: [PATCH 04/19] C++: Add ExternalAPI library files (for AST and IR).

---
 .../semmle/code/cpp/security/ExternalAPIs.qll | 50 ++++++++++++++++++
 .../implementation/ExternalAPIsSpecific.qll   | 52 +++++++++++++++++++
 .../SafeExternalAPIFunction.qll               | 11 ++++
 .../code/cpp/security/ir/ExternalAPIs.qll     | 50 ++++++++++++++++++
 .../implementation/ExternalAPIsSpecific.qll   | 47 +++++++++++++++++
 .../SafeExternalAPIFunction.qll               | 11 ++++
 6 files changed, 221 insertions(+)
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
 create mode 100644 cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll

diff --git a/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll b/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
new file mode 100644
index 000000000000..c3baccb8420d
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
@@ -0,0 +1,50 @@
+/**
+ * Definitions for reasoning about untrusted data used in APIs defined outside the
+ * database.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.Taint
+import implementation.ExternalAPIsSpecific
+
+/** A node representing untrusted data being passed to an external API. */
+class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
+  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
+
+  /** Gets a source of untrusted data which is passed to this external API data node. */
+  DataFlow::Node getAnUntrustedSource() {
+    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
+  }
+}
+
+private newtype TExternalAPI =
+  TExternalAPIParameter(Function f, int index) {
+    exists(UntrustedExternalAPIDataNode n |
+      f = n.getExternalFunction() and
+      index = n.getIndex()
+    )
+  }
+
+/** An external API which is used with untrusted data. */
+class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
+  /** Gets a possibly untrusted use of this external API. */
+  UntrustedExternalAPIDataNode getUntrustedDataNode() {
+    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
+  }
+
+  /** Gets the number of untrusted sources used with this external API. */
+  int getNumberOfUntrustedSources() {
+    result = count(getUntrustedDataNode().getAnUntrustedSource())
+  }
+
+  /** Gets a textual representation of this element. */
+  string toString() {
+    exists(Function f, int index, string indexString |
+      if index = -1 then indexString = "qualifier" else indexString = "param " + index
+    |
+      this = TExternalAPIParameter(f, index) and
+      result = f.toString() + " [" + indexString + "]"
+    )
+  }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
new file mode 100644
index 000000000000..3dbb245abdc2
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
@@ -0,0 +1,52 @@
+import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.models.interfaces.FlowSource
+import semmle.code.cpp.models.interfaces.DataFlow
+import SafeExternalAPIFunction
+
+/** A node representing untrusted data being passed to an external API. */
+class ExternalAPIDataNode extends DataFlow::Node {
+  Call call;
+  int i;
+
+  ExternalAPIDataNode() {
+    // Argument to call to a function
+    (
+      this.asExpr() = call.getArgument(i)
+      or
+      i = -1 and this.asExpr() = call.getQualifier()
+    ) and
+    exists(Function f |
+      f = call.getTarget() and
+      // Defined outside the source archive
+      not f.hasDefinition() and
+      // Not already modeled as a dataflow or taint step
+      not f instanceof DataFlowFunction and
+      not f instanceof TaintFunction and
+      // Not a call to a known safe external API
+      not f instanceof SafeExternalAPIFunction
+    )
+  }
+
+  /** Gets the called API `Function`. */
+  Function getExternalFunction() { result = call.getTarget() }
+
+  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
+  int getIndex() { result = i }
+
+  /** Gets the description of the function being called. */
+  string getFunctionDescription() { result = getExternalFunction().toString() }
+}
+
+/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
+class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(RemoteFlowFunction remoteFlow |
+      remoteFlow = source.asExpr().(Call).getTarget() and
+      remoteFlow.hasRemoteFlowSource(_, _)
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
new file mode 100644
index 000000000000..d46229b718d9
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
@@ -0,0 +1,11 @@
+private import cpp
+
+/**
+ * A `Function` that is considered a "safe" external API from a security perspective.
+ */
+abstract class SafeExternalAPIFunction extends Function { }
+
+/** The default set of "safe" external APIs. */
+private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
+  DefaultSafeExternalAPIFunction() { this.hasGlobalName(["strcmp", "strlen", "memcmp"]) }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll b/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
new file mode 100644
index 000000000000..c3baccb8420d
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
@@ -0,0 +1,50 @@
+/**
+ * Definitions for reasoning about untrusted data used in APIs defined outside the
+ * database.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.Taint
+import implementation.ExternalAPIsSpecific
+
+/** A node representing untrusted data being passed to an external API. */
+class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
+  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
+
+  /** Gets a source of untrusted data which is passed to this external API data node. */
+  DataFlow::Node getAnUntrustedSource() {
+    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
+  }
+}
+
+private newtype TExternalAPI =
+  TExternalAPIParameter(Function f, int index) {
+    exists(UntrustedExternalAPIDataNode n |
+      f = n.getExternalFunction() and
+      index = n.getIndex()
+    )
+  }
+
+/** An external API which is used with untrusted data. */
+class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
+  /** Gets a possibly untrusted use of this external API. */
+  UntrustedExternalAPIDataNode getUntrustedDataNode() {
+    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
+  }
+
+  /** Gets the number of untrusted sources used with this external API. */
+  int getNumberOfUntrustedSources() {
+    result = count(getUntrustedDataNode().getAnUntrustedSource())
+  }
+
+  /** Gets a textual representation of this element. */
+  string toString() {
+    exists(Function f, int index, string indexString |
+      if index = -1 then indexString = "qualifier" else indexString = "param " + index
+    |
+      this = TExternalAPIParameter(f, index) and
+      result = f.toString() + " [" + indexString + "]"
+    )
+  }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
new file mode 100644
index 000000000000..f1874a6f381a
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
@@ -0,0 +1,47 @@
+import semmle.code.cpp.ir.dataflow.TaintTracking
+private import semmle.code.cpp.security.FlowSources
+private import semmle.code.cpp.models.interfaces.DataFlow
+import SafeExternalAPIFunction
+
+/** A node representing untrusted data being passed to an external API. */
+class ExternalAPIDataNode extends DataFlow::Node {
+  Call call;
+  int i;
+
+  ExternalAPIDataNode() {
+    // Argument to call to a function
+    (
+      this.asExpr() = call.getArgument(i)
+      or
+      i = -1 and this.asExpr() = call.getQualifier()
+    ) and
+    exists(Function f |
+      f = call.getTarget() and
+      // Defined outside the source archive
+      not f.hasDefinition() and
+      // Not already modeled as a dataflow or taint step
+      not f instanceof DataFlowFunction and
+      not f instanceof TaintFunction and
+      // Not a call to a known safe external API
+      not f instanceof SafeExternalAPIFunction
+    )
+  }
+
+  /** Gets the called API `Function`. */
+  Function getExternalFunction() { result = call.getTarget() }
+
+  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
+  int getIndex() { result = i }
+
+  /** Gets the description of the function being called. */
+  string getFunctionDescription() { result = getExternalFunction().toString() }
+}
+
+/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
+class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfigIR" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
new file mode 100644
index 000000000000..d46229b718d9
--- /dev/null
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
@@ -0,0 +1,11 @@
+private import cpp
+
+/**
+ * A `Function` that is considered a "safe" external API from a security perspective.
+ */
+abstract class SafeExternalAPIFunction extends Function { }
+
+/** The default set of "safe" external APIs. */
+private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
+  DefaultSafeExternalAPIFunction() { this.hasGlobalName(["strcmp", "strlen", "memcmp"]) }
+}

From 5ad18eb7487dae2a3d405c5b789741c5199a2fba Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Mon, 16 Nov 2020 22:36:23 +0100
Subject: [PATCH 05/19] C++: Add ExternalAPI query files (for AST and IR).

---
 .../ExternalAPIsUsedWithUntrustedData.ql      | 17 ++++++++++++++++
 .../IRExternalAPIsUsedWithUntrustedData.ql    | 17 ++++++++++++++++
 .../CWE-020/IRUntrustedDataToExternalAPI.ql   | 20 +++++++++++++++++++
 .../CWE/CWE-020/UntrustedDataToExternalAPI.ql | 20 +++++++++++++++++++
 4 files changed, 74 insertions(+)
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
new file mode 100644
index 000000000000..5faf2957cb13
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
@@ -0,0 +1,17 @@
+/**
+ * @name Frequency counts for external APIs that are used with untrusted data
+ * @description This reports the external APIs that are used with untrusted data, along with how
+ *              frequently the API is called, and how many unique sources of untrusted data flow
+ *              to it.
+ * @id cpp/count-untrusted-data-external-api
+ * @kind table
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.security.ExternalAPIs
+
+from ExternalAPIUsedWithUntrustedData externalAPI
+select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
+  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
+    numberOfUntrustedSources desc
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql b/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql
new file mode 100644
index 000000000000..3e7f453d8adb
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql
@@ -0,0 +1,17 @@
+/**
+ * @name Frequency counts for external APIs that are used with untrusted data
+ * @description This reports the external APIs that are used with untrusted data, along with how
+ *              frequently the API is called, and how many unique sources of untrusted data flow
+ *              to it.
+ * @id cpp/count-untrusted-data-external-api-ir
+ * @kind table
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.security.ir.ExternalAPIs
+
+from ExternalAPIUsedWithUntrustedData externalAPI
+select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
+  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
+    numberOfUntrustedSources desc
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
new file mode 100644
index 000000000000..ac9ba6671e02
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
@@ -0,0 +1,20 @@
+/**
+ * @name Untrusted data passed to external API
+ * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
+ * @id cpp/untrusted-data-to-external-api-ir
+ * @kind path-problem
+ * @precision low
+ * @problem.severity error
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import semmle.code.cpp.security.ir.ExternalAPIs
+import DataFlow::PathGraph
+
+from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
+    " with untrusted data from $@.", source, source.toString()
diff --git a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
new file mode 100644
index 000000000000..c59c3487fc4e
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
@@ -0,0 +1,20 @@
+/**
+ * @name Untrusted data passed to external API
+ * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
+ * @id cpp/untrusted-data-to-external-api
+ * @kind path-problem
+ * @precision low
+ * @problem.severity error
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.security.ExternalAPIs
+import DataFlow::PathGraph
+
+from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
+    " with untrusted data from $@.", source, source.toString()

From 5c9b8f1cffee273ae881aa790fd5f0b7f67d98e6 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Mon, 16 Nov 2020 22:36:55 +0100
Subject: [PATCH 06/19] C++: Update sync-identical-files.

---
 config/identical-files.json | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/config/identical-files.json b/config/identical-files.json
index 9f0e0a132551..5845b3781bbd 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -358,6 +358,14 @@
     "cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
+  "C++ ExternalAPIs": [
+    "cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll",
+    "cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll"
+  ],
+  "C++ SafeExternalAPIFunction": [
+    "cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll",
+    "cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll"
+  ],
   "XML": [
     "cpp/ql/src/semmle/code/cpp/XML.qll",
     "csharp/ql/src/semmle/code/csharp/XML.qll",

From c3c29b8dd0bc63afc0659ac07d3f3ef4be454242 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 09:09:19 +0100
Subject: [PATCH 07/19] C++: Add qldoc to new library files.

---
 .../code/cpp/security/implementation/ExternalAPIsSpecific.qll | 4 ++++
 .../cpp/security/implementation/SafeExternalAPIFunction.qll   | 4 ++++
 .../cpp/security/ir/implementation/ExternalAPIsSpecific.qll   | 4 ++++
 .../security/ir/implementation/SafeExternalAPIFunction.qll    | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
index 3dbb245abdc2..788baeddbffd 100644
--- a/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
@@ -1,3 +1,7 @@
+/**
+ * Provides AST-specific definitions for use in the `ExternalAPI` library.
+ */
+
 import semmle.code.cpp.dataflow.TaintTracking
 import semmle.code.cpp.models.interfaces.FlowSource
 import semmle.code.cpp.models.interfaces.DataFlow
diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
index d46229b718d9..01312ada7bae 100644
--- a/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
@@ -1,3 +1,7 @@
+/**
+ * Provides a class for modeling external functions that are "safe" from a security perspective.
+ */
+
 private import cpp
 
 /**
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
index f1874a6f381a..10d1728aa019 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
@@ -1,3 +1,7 @@
+/**
+ * Provides IR-specific definitions for use in the `ExternalAPI` library.
+ */
+
 import semmle.code.cpp.ir.dataflow.TaintTracking
 private import semmle.code.cpp.security.FlowSources
 private import semmle.code.cpp.models.interfaces.DataFlow
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
index d46229b718d9..01312ada7bae 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
@@ -1,3 +1,7 @@
+/**
+ * Provides a class for modeling external functions that are "safe" from a security perspective.
+ */
+
 private import cpp
 
 /**

From 3b8580efafe6b6f7a1c9b7e37f62fd0ce475277f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 11:55:24 +0100
Subject: [PATCH 08/19] C++: Add qhelp and example files (modeled after the
 Java examples).

---
 .../CWE/CWE-020/ExternalAPISinkExample.cpp    | 13 +++++
 .../CWE-020/ExternalAPITaintStepExample.cpp   | 13 +++++
 .../ExternalAPIsUsedWithUntrustedData.qhelp   | 48 ++++++++++++++++
 .../CWE-020/UntrustedDataToExternalAPI.qhelp  | 57 +++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp

diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp
new file mode 100644
index 000000000000..18779db9ceb0
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp
@@ -0,0 +1,13 @@
+#include <cstdio>
+
+void do_get(FILE* request, FILE* response) {
+  char page[1024];
+  fgets(page, 1024, request);
+
+  char buffer[1024];
+  strcat(buffer, "The page \"");
+  strcat(buffer, page);
+  strcat(buffer, "\" was not found.");
+
+  fputs(buffer, response);
+}
\ No newline at end of file
diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp
new file mode 100644
index 000000000000..e737ae9b1de7
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp
@@ -0,0 +1,13 @@
+#include <cstdio>
+
+void do_get(FILE* request, FILE* response) {
+  char user_id[1024];
+  fgets(user_id, 1024, request);
+
+  char buffer[1024];
+  strcat(buffer, "SELECT * FROM user WHERE user_id='");
+  strcat(buffer, user_id);
+  strcat(buffer, "'");
+
+  // ...
+}
\ No newline at end of file
diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
new file mode 100644
index 000000000000..72e49d39bdd1
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
+unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
+may be relevant for security analysis of this application.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not
+modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
+third party dependencies or from internal dependencies. The query will report the function name, along with
+either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
+untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
+function call.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, no action is required.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
+  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>If the query were to return the API <code>fputs [param 1]</code> 
+then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
+consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
+
+<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
+reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>
\ No newline at end of file
diff --git a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
new file mode 100644
index 000000000000..9a6f2e3cbdb6
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
@@ -0,0 +1,57 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
+as a taint step in the default taint library. External APIs may be from the
+C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
+untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
+  that the result is a false positive because this data is sanitized.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
+  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
+  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
+<code>fputs</code> external API:</p>
+
+<sample src="ExternalAPISinkExample.cpp" />
+
+<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
+and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
+some existing sanitization.</p>
+
+<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
+
+<sample src="ExternalAPITaintStepExample.cpp" />
+
+<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
+not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
+re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
+will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>
\ No newline at end of file

From c37093f4bc9fddea9e4e163f0951493f25457747 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 12:28:31 +0100
Subject: [PATCH 09/19] C++: Add copies of qhelp files for IR.

---
 .../IRExternalAPIsUsedWithUntrustedData.qhelp | 48 ++++++++++++++++
 .../IRUntrustedDataToExternalAPI.qhelp        | 57 +++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp
 create mode 100644 cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp

diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp b/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp
new file mode 100644
index 000000000000..72e49d39bdd1
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp
@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
+unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
+may be relevant for security analysis of this application.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not
+modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
+third party dependencies or from internal dependencies. The query will report the function name, along with
+either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
+untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
+function call.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, no action is required.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
+  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>If the query were to return the API <code>fputs [param 1]</code> 
+then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
+consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
+
+<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
+reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>
\ No newline at end of file
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
new file mode 100644
index 000000000000..9a6f2e3cbdb6
--- /dev/null
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
@@ -0,0 +1,57 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
+as a taint step in the default taint library. External APIs may be from the
+C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
+untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
+  that the result is a false positive because this data is sanitized.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
+  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
+  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
+<code>fputs</code> external API:</p>
+
+<sample src="ExternalAPISinkExample.cpp" />
+
+<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
+and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
+some existing sanitization.</p>
+
+<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
+
+<sample src="ExternalAPITaintStepExample.cpp" />
+
+<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
+not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
+re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
+will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>
\ No newline at end of file

From 5d2b85fcf56e64db0874255705fe5fa87748fe43 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 13:02:28 +0100
Subject: [PATCH 10/19] Update
 cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll

Co-authored-by: hubwriter <hubwriter@github.com>
---
 cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
index 764a7dab5dca..a47bd7240bef 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
@@ -1,5 +1,5 @@
 /**
- * Provides an implementation class modelling the POSIX function `getenv`.
+ * Provides an implementation class modeling the POSIX function `getenv`.
  */
 import cpp
 import semmle.code.cpp.models.interfaces.FlowSource
@@ -19,4 +19,4 @@ class Getenv extends LocalFlowFunction {
     ) and
     description = "an environment variable"
   }
-}
\ No newline at end of file
+}

From eabc69b98e4e7d727451b20b0a195b3d7b9ebc6f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:09:25 +0100
Subject: [PATCH 11/19] C++: Autoformat

---
 .../src/semmle/code/cpp/models/implementations/Getenv.qll  | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
index a47bd7240bef..9761a4293a8e 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll
@@ -1,6 +1,7 @@
 /**
  * Provides an implementation class modeling the POSIX function `getenv`.
  */
+
 import cpp
 import semmle.code.cpp.models.interfaces.FlowSource
 
@@ -8,11 +9,9 @@ import semmle.code.cpp.models.interfaces.FlowSource
  * The POSIX function `getenv`.
  */
 class Getenv extends LocalFlowFunction {
-  Getenv() {
-    this.hasGlobalName("getenv")
-  }
+  Getenv() { this.hasGlobalName("getenv") }
 
-  override predicate hasLocalFlowSource (FunctionOutput output, string description) {
+  override predicate hasLocalFlowSource(FunctionOutput output, string description) {
     (
       output.isReturnValueDeref() or
       output.isReturnValue()

From dea16d4d62ae529bd739c06d3123dd88fa5891f2 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:11:51 +0100
Subject: [PATCH 12/19] QLDoc/C++: Rename {IR}ExternalAPIsUsedWithUntrustedData
 to {IR}CountUntrustedDataToExternalAPI

---
 ...hUntrustedData.qhelp => CountUntrustedDataToExternalAPI.qhelp} | 0
 ...sedWithUntrustedData.ql => CountUntrustedDataToExternalAPI.ql} | 0
 ...ntrustedData.qhelp => IRCountUntrustedDataToExternalAPI.qhelp} | 0
 ...dWithUntrustedData.ql => IRCountUntrustedDataToExternalAPI.ql} | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename cpp/ql/src/Security/CWE/CWE-020/{ExternalAPIsUsedWithUntrustedData.qhelp => CountUntrustedDataToExternalAPI.qhelp} (100%)
 rename cpp/ql/src/Security/CWE/CWE-020/{ExternalAPIsUsedWithUntrustedData.ql => CountUntrustedDataToExternalAPI.ql} (100%)
 rename cpp/ql/src/Security/CWE/CWE-020/{IRExternalAPIsUsedWithUntrustedData.qhelp => IRCountUntrustedDataToExternalAPI.qhelp} (100%)
 rename cpp/ql/src/Security/CWE/CWE-020/{IRExternalAPIsUsedWithUntrustedData.ql => IRCountUntrustedDataToExternalAPI.ql} (100%)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp b/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.qhelp
similarity index 100%
rename from cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
rename to cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.qhelp
diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql b/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql
similarity index 100%
rename from cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
rename to cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp b/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.qhelp
similarity index 100%
rename from cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.qhelp
rename to cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.qhelp
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql b/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql
similarity index 100%
rename from cpp/ql/src/Security/CWE/CWE-020/IRExternalAPIsUsedWithUntrustedData.ql
rename to cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql

From 4cb25d8e18bfd283c67474486498a9d1e39e0803 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:12:39 +0100
Subject: [PATCH 13/19] C++: Add isParameterDerefOrQualifierObject helper
 predicate to FunctionInput and FunctionOutput.

---
 .../interfaces/FunctionInputsAndOutputs.qll   | 20 +++++++++++++++++++
 .../semmle/code/cpp/security/FlowSources.qll  |  4 ++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll
index 9804cd6aff75..195cbcb63b5c 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FunctionInputsAndOutputs.qll
@@ -132,6 +132,16 @@ class FunctionInput extends TFunctionInput {
    * part of itself, or one of its other inputs.
    */
   predicate isReturnValueDeref() { none() }
+
+  /**
+   * Holds if `i >= 0` and `isParameterDeref(i)` holds for this is value, or
+   * if `i = -1` and `isQualifierObject()` holds for this value.
+   */
+  final predicate isParameterDerefOrQualifierObject(ParameterIndex i) {
+    i >= 0 and this.isParameterDeref(i)
+    or
+    i = -1 and this.isQualifierObject()
+  }
 }
 
 /**
@@ -370,6 +380,16 @@ class FunctionOutput extends TFunctionOutput {
    * DEPRECATED: Use `isReturnValueDeref()` instead.
    */
   deprecated final predicate isOutReturnPointer() { isReturnValueDeref() }
+
+  /**
+   * Holds if `i >= 0` and `isParameterDeref(i)` holds for this is the value, or
+   * if `i = -1` and `isQualifierObject()` holds for this value.
+   */
+  final predicate isParameterDerefOrQualifierObject(ParameterIndex i) {
+    i >= 0 and this.isParameterDeref(i)
+    or
+    i = -1 and this.isQualifierObject()
+  }
 }
 
 /**
diff --git a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
index 4fe5d1bb8736..c8bad67352bf 100644
--- a/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/FlowSources.qll
@@ -46,7 +46,7 @@ private class RemoteParameterSource extends RemoteFlowSource {
       asInstruction() = instr and
       instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
       func.hasRemoteFlowSource(output, sourceType) and
-      output.isParameterDeref(instr.getIndex())
+      output.isParameterDerefOrQualifierObject(instr.getIndex())
     )
   }
 
@@ -80,7 +80,7 @@ private class LocalParameterSource extends LocalFlowSource {
       asInstruction() = instr and
       instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
       func.hasLocalFlowSource(output, sourceType) and
-      output.isParameterDeref(instr.getIndex())
+      output.isParameterDerefOrQualifierObject(instr.getIndex())
     )
   }
 

From d1272d3a799baf16c9cd181e56daebf1083d4743 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:14:45 +0100
Subject: [PATCH 14/19] C++: Use strictcount instead of count.

---
 cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll    | 2 +-
 cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll b/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
index c3baccb8420d..f7fc4ee8c508 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
@@ -35,7 +35,7 @@ class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
 
   /** Gets the number of untrusted sources used with this external API. */
   int getNumberOfUntrustedSources() {
-    result = count(getUntrustedDataNode().getAnUntrustedSource())
+    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
   }
 
   /** Gets a textual representation of this element. */
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll b/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
index c3baccb8420d..f7fc4ee8c508 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
@@ -35,7 +35,7 @@ class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
 
   /** Gets the number of untrusted sources used with this external API. */
   int getNumberOfUntrustedSources() {
-    result = count(getUntrustedDataNode().getAnUntrustedSource())
+    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
   }
 
   /** Gets a textual representation of this element. */

From d93d3c869930ba5a62d4fc067236c9a54824810f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:16:13 +0100
Subject: [PATCH 15/19] C++: Use the getSourceType predicate on
 RemoteFlowSources for better alert messages.

---
 .../src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
index ac9ba6671e02..b51a099193b5 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
@@ -11,10 +11,11 @@
 import cpp
 import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.security.ir.ExternalAPIs
+import semmle.code.cpp.security.FlowSources
 import DataFlow::PathGraph
 
 from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
 select sink, source, sink,
   "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
-    " with untrusted data from $@.", source, source.toString()
+    " with untrusted data from $@.", source, source.getNode().(RemoteFlowSource).getSourceType()

From 52bbb326ca71cde3665c5dba3f1644a0018b1c42 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 17 Nov 2020 16:16:53 +0100
Subject: [PATCH 16/19] QLDoc: Wrap lines and disambiguate explanation.

---
 .../Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp   | 4 +++-
 .../src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
index 9a6f2e3cbdb6..df14d71947b6 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
@@ -4,7 +4,9 @@
 <qhelp>
 <overview>
 <p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.</p>
+external APIs that use untrusted data. The results are not filtered. This makes it possible to audit all examples.
+The query provides data for security reviews of the application and you can also use it to identify external APIs
+that should be modeled as either taint steps, or sinks for specific problems.</p>
 
 <p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
 as a taint step in the default taint library. External APIs may be from the
diff --git a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
index 9a6f2e3cbdb6..df14d71947b6 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
+++ b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
@@ -4,7 +4,9 @@
 <qhelp>
 <overview>
 <p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.</p>
+external APIs that use untrusted data. The results are not filtered. This makes it possible to audit all examples.
+The query provides data for security reviews of the application and you can also use it to identify external APIs
+that should be modeled as either taint steps, or sinks for specific problems.</p>
 
 <p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
 as a taint step in the default taint library. External APIs may be from the

From f16591dffc4a538a32700c077487fcae8fcf26ca Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 18 Nov 2020 09:18:14 +0100
Subject: [PATCH 17/19] C++: Respond to qhelp review comments.

---
 .../src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp | 2 +-
 .../src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
index df14d71947b6..edadb4b14ec2 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp
@@ -4,7 +4,7 @@
 <qhelp>
 <overview>
 <p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered. This makes it possible to audit all examples.
+external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
 The query provides data for security reviews of the application and you can also use it to identify external APIs
 that should be modeled as either taint steps, or sinks for specific problems.</p>
 
diff --git a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
index df14d71947b6..edadb4b14ec2 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
+++ b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
@@ -4,7 +4,7 @@
 <qhelp>
 <overview>
 <p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered. This makes it possible to audit all examples.
+external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
 The query provides data for security reviews of the application and you can also use it to identify external APIs
 that should be modeled as either taint steps, or sinks for specific problems.</p>
 

From 09c5caa3bd72cdf25c9f069126da7199e5000c69 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 18 Nov 2020 09:28:50 +0100
Subject: [PATCH 18/19] C++: Move ExternalAPI files into query directory to
 prevent out-of-tree use.

---
 config/identical-files.json                               | 8 ++++----
 .../CWE/CWE-020/CountUntrustedDataToExternalAPI.ql        | 2 +-
 .../security => Security/CWE/CWE-020}/ExternalAPIs.qll    | 2 +-
 .../CWE/CWE-020}/ExternalAPIsSpecific.qll                 | 0
 .../CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql      | 2 +-
 .../Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql  | 2 +-
 .../CWE/CWE-020}/SafeExternalAPIFunction.qll              | 1 +
 .../Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql    | 2 +-
 .../security => Security/CWE/CWE-020}/ir/ExternalAPIs.qll | 2 +-
 .../CWE/CWE-020/ir}/ExternalAPIsSpecific.qll              | 0
 .../CWE/CWE-020/ir}/SafeExternalAPIFunction.qll           | 1 +
 11 files changed, 12 insertions(+), 10 deletions(-)
 rename cpp/ql/src/{semmle/code/cpp/security => Security/CWE/CWE-020}/ExternalAPIs.qll (97%)
 rename cpp/ql/src/{semmle/code/cpp/security/implementation => Security/CWE/CWE-020}/ExternalAPIsSpecific.qll (100%)
 rename cpp/ql/src/{semmle/code/cpp/security/implementation => Security/CWE/CWE-020}/SafeExternalAPIFunction.qll (89%)
 rename cpp/ql/src/{semmle/code/cpp/security => Security/CWE/CWE-020}/ir/ExternalAPIs.qll (97%)
 rename cpp/ql/src/{semmle/code/cpp/security/ir/implementation => Security/CWE/CWE-020/ir}/ExternalAPIsSpecific.qll (100%)
 rename cpp/ql/src/{semmle/code/cpp/security/ir/implementation => Security/CWE/CWE-020/ir}/SafeExternalAPIFunction.qll (89%)

diff --git a/config/identical-files.json b/config/identical-files.json
index 5845b3781bbd..5a8efc3405a9 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -359,12 +359,12 @@
     "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
   "C++ ExternalAPIs": [
-    "cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll",
-    "cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll"
+    "cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll",
+    "cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll"
   ],
   "C++ SafeExternalAPIFunction": [
-    "cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll",
-    "cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll"
+    "cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll",
+    "cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll"
   ],
   "XML": [
     "cpp/ql/src/semmle/code/cpp/XML.qll",
diff --git a/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql
index 5faf2957cb13..8c75e8da6e2d 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql
+++ b/cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql
@@ -9,7 +9,7 @@
  */
 
 import cpp
-import semmle.code.cpp.security.ExternalAPIs
+import ExternalAPIs
 
 from ExternalAPIUsedWithUntrustedData externalAPI
 select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
diff --git a/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll
similarity index 97%
rename from cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
rename to cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll
index f7fc4ee8c508..29d5b20cfc42 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ExternalAPIs.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll
@@ -6,7 +6,7 @@
 private import cpp
 private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.models.interfaces.Taint
-import implementation.ExternalAPIsSpecific
+import ExternalAPIsSpecific
 
 /** A node representing untrusted data being passed to an external API. */
 class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll
similarity index 100%
rename from cpp/ql/src/semmle/code/cpp/security/implementation/ExternalAPIsSpecific.qll
rename to cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql
index 3e7f453d8adb..4d0c2174809d 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql
@@ -9,7 +9,7 @@
  */
 
 import cpp
-import semmle.code.cpp.security.ir.ExternalAPIs
+import ir.ExternalAPIs
 
 from ExternalAPIUsedWithUntrustedData externalAPI
 select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
diff --git a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
index b51a099193b5..e4f0cc7883d5 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
+++ b/cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql
@@ -10,7 +10,7 @@
 
 import cpp
 import semmle.code.cpp.ir.dataflow.TaintTracking
-import semmle.code.cpp.security.ir.ExternalAPIs
+import ir.ExternalAPIs
 import semmle.code.cpp.security.FlowSources
 import DataFlow::PathGraph
 
diff --git a/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
similarity index 89%
rename from cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
rename to cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
index 01312ada7bae..dc5ca91cebd6 100644
--- a/cpp/ql/src/semmle/code/cpp/security/implementation/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
@@ -3,6 +3,7 @@
  */
 
 private import cpp
+private import semmle.code.cpp.models.implementations.Pure
 
 /**
  * A `Function` that is considered a "safe" external API from a security perspective.
diff --git a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
index c59c3487fc4e..ca6d2d00e8c3 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
+++ b/cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
@@ -10,7 +10,7 @@
 
 import cpp
 import semmle.code.cpp.dataflow.TaintTracking
-import semmle.code.cpp.security.ExternalAPIs
+import ExternalAPIs
 import DataFlow::PathGraph
 
 from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll b/cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll
similarity index 97%
rename from cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
rename to cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll
index f7fc4ee8c508..29d5b20cfc42 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ir/ExternalAPIs.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll
@@ -6,7 +6,7 @@
 private import cpp
 private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.models.interfaces.Taint
-import implementation.ExternalAPIsSpecific
+import ExternalAPIsSpecific
 
 /** A node representing untrusted data being passed to an external API. */
 class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll b/cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIsSpecific.qll
similarity index 100%
rename from cpp/ql/src/semmle/code/cpp/security/ir/implementation/ExternalAPIsSpecific.qll
rename to cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIsSpecific.qll
diff --git a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
similarity index 89%
rename from cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
rename to cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
index 01312ada7bae..dc5ca91cebd6 100644
--- a/cpp/ql/src/semmle/code/cpp/security/ir/implementation/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
@@ -3,6 +3,7 @@
  */
 
 private import cpp
+private import semmle.code.cpp.models.implementations.Pure
 
 /**
  * A `Function` that is considered a "safe" external API from a security perspective.

From 715f2333609c65fae9920fa038ed9650c4167ecd Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 18 Nov 2020 12:47:33 +0100
Subject: [PATCH 19/19] C++: Add a new model class describing pure memory
 functions, and use this new model in DefaultSafeExternalAPIFunction.

---
 .../CWE/CWE-020/SafeExternalAPIFunction.qll   |  6 ++-
 .../CWE-020/ir/SafeExternalAPIFunction.qll    |  6 ++-
 .../code/cpp/models/implementations/Pure.qll  | 49 +++++++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
index dc5ca91cebd6..cdcebe6884c1 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
@@ -12,5 +12,9 @@ abstract class SafeExternalAPIFunction extends Function { }
 
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
-  DefaultSafeExternalAPIFunction() { this.hasGlobalName(["strcmp", "strlen", "memcmp"]) }
+  DefaultSafeExternalAPIFunction() {
+    this instanceof PureStrFunction or
+    this instanceof StrLenFunction or
+    this instanceof PureMemFunction
+  }
 }
diff --git a/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
index dc5ca91cebd6..cdcebe6884c1 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
+++ b/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
@@ -12,5 +12,9 @@ abstract class SafeExternalAPIFunction extends Function { }
 
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
-  DefaultSafeExternalAPIFunction() { this.hasGlobalName(["strcmp", "strlen", "memcmp"]) }
+  DefaultSafeExternalAPIFunction() {
+    this instanceof PureStrFunction or
+    this instanceof StrLenFunction or
+    this instanceof PureMemFunction
+  }
 }
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index da69cc2ac819..5c072ca33705 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -3,6 +3,7 @@ import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
+/** Pure string functions. */
 class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
   PureStrFunction() {
     hasGlobalOrStdName([
@@ -58,6 +59,7 @@ class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideE
   }
 }
 
+/** String standard `strlen` function, and related functions for computing string lengths. */
 class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   StrLenFunction() {
     hasGlobalOrStdName(["strlen", "strnlen", "wcslen"])
@@ -91,6 +93,7 @@ class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   }
 }
 
+/** Pure functions. */
 class PureFunction extends TaintFunction, SideEffectFunction {
   PureFunction() { hasGlobalOrStdName(["abs", "labs"]) }
 
@@ -106,3 +109,49 @@ class PureFunction extends TaintFunction, SideEffectFunction {
 
   override predicate hasOnlySpecificWriteSideEffects() { any() }
 }
+
+/** Pure raw-memory functions. */
+class PureMemFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
+  PureMemFunction() { hasGlobalOrStdName(["memchr", "memrchr", "rawmemchr", "memcmp", "memmem"]) }
+
+  override predicate hasArrayInput(int bufParam) {
+    getParameter(bufParam).getUnspecifiedType() instanceof PointerType
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    exists(ParameterIndex i |
+      input.isParameter(i) and
+      exists(getParameter(i))
+      or
+      input.isParameterDeref(i) and
+      getParameter(i).getUnspecifiedType() instanceof PointerType
+    ) and
+    (
+      output.isReturnValueDeref() and
+      getUnspecifiedType() instanceof PointerType
+      or
+      output.isReturnValue()
+    )
+  }
+
+  override predicate parameterNeverEscapes(int i) {
+    getParameter(i).getUnspecifiedType() instanceof PointerType and
+    not parameterEscapesOnlyViaReturn(i)
+  }
+
+  override predicate parameterEscapesOnlyViaReturn(int i) {
+    i = 0 and
+    getUnspecifiedType() instanceof PointerType
+  }
+
+  override predicate parameterIsAlwaysReturned(int i) { none() }
+
+  override predicate hasOnlySpecificReadSideEffects() { any() }
+
+  override predicate hasOnlySpecificWriteSideEffects() { any() }
+
+  override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
+    getParameter(i).getUnspecifiedType() instanceof PointerType and
+    buffer = true
+  }
+}