From 8a65dd2cd6551a8b396825960518faa752eb9d47 Mon Sep 17 00:00:00 2001
From: "lcartey@github.com" <lcartey@github.com>
Date: Tue, 11 Aug 2020 15:28:06 +0100
Subject: [PATCH] Java: Address review comments

---
 .../CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp     | 4 ++--
 .../CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql        | 4 ++--
 .../Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp   | 4 ++--
 java/ql/src/semmle/code/java/security/ExternalAPIs.qll      | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp b/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
index 4f04b90fa49..ed0abf51cb5 100644
--- a/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
+++ b/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp
@@ -4,11 +4,11 @@
 <qhelp>
 <overview>
 <p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-all external APIs which are used with untrusted data, along with how frequently the API is used, and how many 
+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 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 method call to a method which is not defined in the source code, not overridden 
+<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden 
 in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the 
 Java standard library, third party dependencies or from internal dependencies. The query will report the method 
 signature with a fully qualified name, along with either <code>[param x]</code>, where <code>x</code> indicates the 
diff --git a/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql b/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
index bbe627e0490..6bb7c6c6c51 100644
--- a/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
+++ b/java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
@@ -1,6 +1,6 @@
 /**
- * @name Frequency counts for external APIs which are used with untrusted data
- * @description This reports the external APIs which are used with untrusted data, along with how
+ * @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 java/count-untrusted-data-external-api
diff --git a/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp b/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
index 02f754cfd56..3a89f3a7af0 100644
--- a/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
+++ b/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp
@@ -8,7 +8,7 @@ all uses of external APIs with untrusted data for review. This query has a delib
 and is designed to help security reviews for the application, as well as helping 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 method call to a method which is not defined in the source code, not overridden 
+<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden 
 in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the
 Java standard library, third party dependencies or from internal dependencies. The query will report uses of 
 untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
@@ -28,7 +28,7 @@ untrusted data in either the qualifier or as one of the arguments of external AP
 </ul>
 
 <p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIMethod</code> 
-class to exclude known safe external APIs from future analysis.
+class to exclude known safe external APIs from future analysis.</p>
 
 </recommendation>
 <example>
diff --git a/java/ql/src/semmle/code/java/security/ExternalAPIs.qll b/java/ql/src/semmle/code/java/security/ExternalAPIs.qll
index 2039a3f3f14..ca81a4b0b8b 100644
--- a/java/ql/src/semmle/code/java/security/ExternalAPIs.qll
+++ b/java/ql/src/semmle/code/java/security/ExternalAPIs.qll
@@ -9,7 +9,7 @@ import semmle.code.java.dataflow.TaintTracking
 
 module ExternalAPIs {
   /**
-   * A `Method` which is considered a "safe" external API from a security perspective.
+   * A `Method` that is considered a "safe" external API from a security perspective.
    */
   abstract class SafeExternalAPIMethod extends Method { }
 
@@ -24,7 +24,7 @@ module ExternalAPIs {
       or
       getQualifiedName() = "Objects.equals"
       or
-      getDeclaringType().getQualifiedName() = "java.lang.String" and getName() = "equals"
+      getDeclaringType() instanceof TypeString and getName() = "equals"
       or
       getDeclaringType().hasQualifiedName("com.google.common.base", "Preconditions")
       or
@@ -76,7 +76,7 @@ module ExternalAPIs {
         m.getASourceOverriddenMethod() = call.getCallee().getSourceDeclaration() and
         m.fromSource()
       ) and
-      // Not already modelled as a taint step
+      // Not already modeled as a taint step
       not exists(DataFlow::Node next | TaintTracking::localTaintStep(this, next)) and
       // Not a call to a known safe external API
       not call.getCallee() instanceof SafeExternalAPIMethod