Skip to content

Commit 56ff8cf

Browse files
lcarteyfelicitymay
andauthored
Apply suggestions from code review
Co-authored-by: Felicity Chapman <[email protected]>
1 parent 6b6172f commit 56ff8cf

File tree

4 files changed

+11
-12
lines changed

4 files changed

+11
-12
lines changed

java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.qhelp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<overview>
66
<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports
77
all external APIs that are used with untrusted data, along with how frequently the API is used, and how many
8-
unique sources of untrusted data flow this API. This query is designed primarily to help identify which APIs
8+
unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs
99
may be relevant for security analysis of this application.</p>
1010

1111
<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden
@@ -40,7 +40,7 @@ consider whether this is an XSS sink. If it is, we should confirm that it is han
4040
<p>If the query were to return the API <code>java.lang.StringBuilder.append(java.lang.String) [param 0]</code>, then this should be
4141
reviewed as a possible taint step, because tainted data would flow from the 0th argument to the qualifier of the call.</p>
4242

43-
<p>Note that both examples are correctly handled with the standard taint tracking library and XSS query.</p>
43+
<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
4444
</example>
4545
<references>
4646

java/ql/src/Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* to it.
66
* @id java/count-untrusted-data-external-api
77
* @kind table
8+
* @tags security external/cwe/cwe-20
89
*/
910

1011
import java

java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
<qhelp>
55
<overview>
66
<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports
7-
all uses of external APIs with untrusted data for review. This query has a deliberately low true positive rate,
8-
and is designed to help security reviews for the application, as well as helping identify external APIs that
9-
should be modeled as either taint steps, or sinks for specific problems.</p>
7+
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>
108

119
<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden
1210
in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the
13-
Java standard library, third party dependencies or from internal dependencies. The query will report uses of
11+
Java standard library, third-party dependencies or from internal dependencies. The query reports uses of
1412
untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
1513

1614
</overview>
@@ -20,10 +18,10 @@ untrusted data in either the qualifier or as one of the arguments of external AP
2018

2119
<ul>
2220
<li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or
23-
that the result is a false positive due to sanitization.</li>
21+
that the result is a false positive because this data is sanitized.</li>
2422
<li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query,
2523
and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
26-
<li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and
24+
<li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and
2725
re-run the query to determine what new results have appeared due to this additional modeling.</li>
2826
</ul>
2927

@@ -39,19 +37,19 @@ class to exclude known safe external APIs from future analysis.</p>
3937
<sample src="ExternalAPISinkExample.java" />
4038

4139
<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled,
42-
and if it is, to confirm that the query reports this particular result, or that the result is false positive due to
40+
and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to
4341
some existing sanitization.</p>
4442

4543
<p>In this second example, again a request parameter is read from <code>HttpServletRequest</code>.</p>
4644

4745
<sample src="ExternalAPITaintStepExample.java" />
4846

4947
<p>If the query reported the call to <code>StringBuilder.append</code> on line 7, this would suggest that this external API is
50-
not currently modeled as a taint step in the taint tracking library. The next step would be to model this as taint step, then
48+
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
5149
re-run the query to determine what additional results might be found. In this example, it seems likely that the result of the
5250
<code>StringBuilder</code> will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
5351

54-
<p>Note that both examples are correctly handled with the standard taint tracking library and XSS query.</p>
52+
<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
5553
</example>
5654
<references>
5755

java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name Untrusted data passed to external API
3-
* @description Data provided remotely is used in this external API.
3+
* @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
44
* @id java/untrusted-data-to-external-api
55
* @kind path-problem
66
* @precision very-low

0 commit comments

Comments
 (0)