From 7730d66d768ba42614e7f728b92df9744389ebe2 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Fri, 27 Nov 2020 12:07:29 +0000 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Shati Patel <42641846+shati-patel@users.noreply.github.com> --- change-notes/2020-11-27-external-api.md | 4 ++++ .../ExternalAPIsUsedWithUntrustedData.qhelp | 14 +++++++------- .../CWE-020/UntrustedDataToExternalAPI.qhelp | 9 +++++---- .../UntrustedDataToUnknownExternalAPI.qhelp | 13 +++++++------ 4 files changed, 23 insertions(+), 17 deletions(-) create mode 100644 change-notes/2020-11-27-external-api.md diff --git a/change-notes/2020-11-27-external-api.md b/change-notes/2020-11-27-external-api.md new file mode 100644 index 00000000000..b65b4da8534 --- /dev/null +++ b/change-notes/2020-11-27-external-api.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* A new query "Untrusted data passed to external API" (`go/untrusted-data-to-external-api`) has been added. The query reports external APIs that use untrusted data. This query is designed primarily to help identify which APIs may be relevant for security analysis of this application. +* A new query "Untrusted data passed to unknown external API" (`go/untrusted-data-to-unknown-external-api`) has been added. The query reports external APIs that use untrusted data and which are not already known to be safe. This query is designed primarily to help identify which APIs may be relevant for security analysis of this application. +* A new query "Frequency counts for external APIs that are used with untrusted data" (`go/count-untrusted-data-external-api`) has been added. The query reports external APIs that use untrusted data. It displays the same results as "Untrusted data passed to external API" (`go/untrusted-data-to-external-api`) but in a table. diff --git a/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp b/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp index 7f23586d701..8b56b9d97da 100644 --- a/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp +++ b/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp @@ -10,10 +10,10 @@ may be relevant for security analysis of this application.
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. Calls made in test files are excluded.
-External APIs may be from the Go standard library, third party dependencies or from internal dependencies.
+External APIs may be from the Go standard library, third party dependencies, or from internal dependencies.
The query will report the fully-qualified method name, along with either [param x],
-where x indicates the position of the parameter receiving the untrusted data or [receiver]
-indicating the untrusted data is used as the receiver of the method call.
x indicates the position of the parameter receiving the untrusted data, or [receiver]
+indicating that the untrusted data is used as the receiver of the method call.
If the query were to return the API fmt.Fprintf [param 2] then we should first consider
-whether this a security relevant sink. In this case, this is writing to a HTTP response, so we should
-consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.
go/reflected-xss).
If the query were to return the API fmt.Sprintf [param 1], then this should be
-reviewed as a possible taint step, because tainted data would flow from the 1st argument to the return value
+reviewed as a possible taint step, because tainted data would flow from the first argument to the return value
of the call.
Note that both examples are correctly handled by the standard taint tracking library and XSS query.
+Note that both examples are correctly handled by the standard taint tracking library and the "Reflected cross-site scripting" query (go/reflected-xss).
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. Calls made in test files are excluded. -External APIs may be from the Go standard library, third-party dependencies or from internal dependencies. +External APIs may be from the Go standard library, third-party dependencies, or from internal dependencies. The query reports uses of untrusted data in either the receiver or as one of the arguments of external APIs.
@@ -38,7 +38,7 @@ class to exclude known safe external APIs from future analysis.This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, +
This is an XSS sink. The "Reflected cross-site scripting" query (go/reflected-xss) 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.
Note that both examples are correctly handled by the standard taint tracking library, the reflected XSS query and SQL -injection query.
+Note that both examples are correctly handled by the standard taint tracking library,
+the "Reflected cross-site scripting" query (go/reflected-xss),
+and the "Database query built from user-controlled sources" query (go/sql-injection).
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 have been filtered. The query provides data for security +external APIs that use untrusted data. The results have been filtered to only report unknown external APIs. The query provides data for security reviews of the application. It can also be used to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.
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. Calls made in test files are excluded. -External APIs may be from the Go standard library, third-party dependencies or from internal dependencies. +External APIs may be from the Go standard library, third-party dependencies, or from internal dependencies. The query reports uses of untrusted data in either the receiver or as one of the arguments of external APIs.
An external API is considered unknown if it is not in a package which has already been modeled, it is not -a sink for an existing query and it is not in a list of external APIs which have been examined and determined +a sink for an existing query, and it is not in a list of external APIs which have been examined and determined to not be a possible source of security vulnerabilities.
This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, +
This is an XSS sink. The "Reflected cross-site scripting" query (go/reflected-xss) 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.
Note that both examples are correctly handled by the standard taint tracking library, the reflected XSS query and SQL -injection query.
+Note that both examples are correctly handled by the standard taint tracking library,
+the "Reflected cross-site scripting" query (go/reflected-xss),
+and the "Database query built from user-controlled sources" query (go/sql-injection).