From dec7967c7a226e54ba167d1055eb1f66ff84ff1b Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 25 Nov 2020 11:28:06 +0000 Subject: [PATCH] Update qhelp files --- .../CWE-020/ExternalAPISinkExample.go | 26 +++++--- .../CWE-020/ExternalAPITaintStepExample.go | 19 +++--- .../ExternalAPIsUsedWithUntrustedData.qhelp | 21 ++++--- .../CWE-020/UntrustedDataToExternalAPI.qhelp | 35 +++++------ .../UntrustedDataToUnknownExternalAPI.qhelp | 61 +++++++++++++++++++ 5 files changed, 119 insertions(+), 43 deletions(-) create mode 100644 ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.qhelp diff --git a/ql/src/Security/CWE-020/ExternalAPISinkExample.go b/ql/src/Security/CWE-020/ExternalAPISinkExample.go index 273da84901c..002732488e1 100644 --- a/ql/src/Security/CWE-020/ExternalAPISinkExample.go +++ b/ql/src/Security/CWE-020/ExternalAPISinkExample.go @@ -1,8 +1,20 @@ -public class XSS extends HttpServlet { - protected void doGet(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { - // BAD: a request parameter is written directly to an error response page - response.sendError(HttpServletResponse.SC_NOT_FOUND, - "The page \"" + request.getParameter("page") + "\" was not found."); - } +package main + +import ( + "fmt" + "net/http" +) + +func serve() { + http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { + r.ParseForm() + username := r.Form.Get("username") + if !isValidUsername(username) { + // BAD: a request parameter is incorporated without validation into the response + fmt.Fprintf(w, "%q is an unknown user", username) + } else { + // TODO: do something exciting + } + }) + http.ListenAndServe(":80", nil) } diff --git a/ql/src/Security/CWE-020/ExternalAPITaintStepExample.go b/ql/src/Security/CWE-020/ExternalAPITaintStepExample.go index 65735194a69..0df976d93c3 100644 --- a/ql/src/Security/CWE-020/ExternalAPITaintStepExample.go +++ b/ql/src/Security/CWE-020/ExternalAPITaintStepExample.go @@ -1,12 +1,13 @@ -public class SQLInjection extends HttpServlet { - protected void doGet(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { +package main - StringBuilder sqlQueryBuilder = new StringBuilder(); - sqlQueryBuilder.append("SELECT * FROM user WHERE user_id='"); - sqlQueryBuilder.append(request.getParameter("user_id")); - sqlQueryBuilder.append("'"); +import ( + "database/sql" + "fmt" + "net/http" +) - // ... - } +func handler(db *sql.DB, req *http.Request) { + q := fmt.Sprintf("SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='%s' ORDER BY PRICE", + req.URL.Query()["category"]) + db.Query(q) } diff --git a/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp b/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp index 73bedfef1c4..e39347034b2 100644 --- a/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp +++ b/ql/src/Security/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp @@ -8,12 +8,12 @@ all external APIs that are used with untrusted data, along with how frequently t 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.

-

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 -Go standard library, third party dependencies or from internal dependencies. The query will report the method -signature with a fully qualified 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.

+

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 also excluded. +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.

@@ -33,12 +33,13 @@ class to exclude known safe external APIs from future analysis.

 -

If the query were to return the API javax.servlet.http.HttpServletResponse.sendError(int, java.lang.String) [param 1] -then we should first consider whether this a security relevant sink. In this case, this is writing to a HTTP response, so we should +

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.

-

If the query were to return the API java.lang.StringBuilder.append(java.lang.String) [param 0], then this should be -reviewed as a possible taint step, because tainted data would flow from the 0th argument to the qualifier of the call.

+

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 +of the call.

Note that both examples are correctly handled by the standard taint tracking library and XSS query.

 diff --git a/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp b/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp index b73f6952946..66dd456777e 100644 --- a/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp +++ b/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp @@ -4,14 +4,14 @@

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.

+external APIs that use untrusted data. The results have very little filtering so that you can audit almost 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.

-

An external API is defined as a call to a function 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 -Go 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.

+

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 also excluded. +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.

 @@ -33,25 +33,26 @@ class to exclude known safe external APIs from future analysis.

 -

In this first example, a request parameter is read from HttpServletRequest and then ultimately used in a call to the -HttpServletResponse.sendError external API:

+

In this first example, a request parameter is read from http.Request and then ultimately used in a call to the +fmt.Fprintf external API:

- +

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.

-

In this second example, again a request parameter is read from HttpServletRequest.

+

In this second example, again a request parameter is read from http.Request.

- + -

If the query reported the call to StringBuilder.append on line 7, 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 the result of the -StringBuilder will be executed as an SQL query, potentially leading to an SQL injection vulnerability.

+

If the query reported the call to fmt.Sprintf, 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, an SQL injection vulnerability +would be reported.

-

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, the reflected XSS query and SQL +injection query.

 diff --git a/ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.qhelp b/ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.qhelp new file mode 100644 index 00000000000..3dc5403f255 --- /dev/null +++ b/ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.qhelp @@ -0,0 +1,61 @@ + + + +

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 +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.

+ +

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 also excluded. +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 +to not be a possible source of security vulnerabilities.

+ + + +

For each result:

+ +
    +
  • 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.
    • +
  • 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.
    • +
+ +

Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod +class to exclude known safe external APIs from future analysis.

+ + + + +

In this first example, a request parameter is read from http.Request and then ultimately used in a call to the +fmt.Fprintf external API:

+ + + +

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.

+ +

In this second example, again a request parameter is read from http.Request.

+ + + +

If the query reported the call to fmt.Sprintf, 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, an SQL injection vulnerability +would be reported.

+ +

Note that both examples are correctly handled by the standard taint tracking library, the reflected XSS query and SQL +injection query.

+ + + + +