hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11730 from smowton/smowton/admin/improve-sql-unescaped-docs

Browse Source 
Java: improve naming and description of SqlUnescaped.ql
This commit is contained in:
Chris Smowton
2023-01-09 12:50:27 +00:00
committed by GitHub
parent c01ce955ba efe23c1da7
commit e9bbb5d7fa
11 changed files with 30 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/ql/lib/semmle/code/java/security/SqlUnescapedLib.qll → java/ql/lib/semmle/code/java/security/SqlConcatenatedLib.qll
View File

@@ -1,4 +1,4 @@
/** Definitions used by `SqlUnescaped.ql`. */
/** Definitions used by `SqlConcatenated.ql`. */
import semmle.code.java.security.ControlledString
import semmle.code.java.dataflow.TaintTracking

2
java/ql/lib/semmle/code/java/security/SqlInjectionQuery.qll
View File

@@ -33,7 +33,7 @@ class QueryInjectionFlowConfig extends TaintTracking::Configuration {
/**
* Implementation of `SqlTainted.ql`. This is extracted to a QLL so that it
* can be excluded from `SqlUnescaped.ql` to avoid overlapping results.
* can be excluded from `SqlConcatenated.ql` to avoid overlapping results.
*/
predicate queryTaintedBy(
QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink

0
java/ql/src/Security/CWE/CWE-089/SqlUnescaped.java → java/ql/src/Security/CWE/CWE-089/SqlConcatenated.java
View File

10
java/ql/src/Security/CWE/CWE-089/SqlUnescaped.qhelp → java/ql/src/Security/CWE/CWE-089/SqlConcatenated.qhelp
View File

@@ -4,8 +4,8 @@
<qhelp>
<overview>
<p>Even when the components of a SQL query are not fully controlled by
a user, it is a vulnerability to concatenate those components into a
SQL query without neutralizing special characters. Perhaps a separate
a user, it is a vulnerability to build the query by directly
concatenating those components. Perhaps a separate
vulnerability will allow the user to gain control of the component. As
well, a user who cannot gain full control of an input might influence
it enough to cause the SQL query to fail to run.</p>
@@ -21,18 +21,18 @@ it enough to cause the SQL query to fail to run.</p>
<p>In the following example, the code runs a simple SQL query in two different ways.</p>
<p>The first way involves building a query, <code>query1</code>, by concatenating the
result of <code>getCategory</code> with some string literals. The result of
result of <code>getCategory</code> with some string literals. The result of
<code>getCategory</code> can include special characters, or
it might be refactored later so that it may return something that contains special characters.</p>
<p>The second way, which shows good practice, involves building a query, <code>query2</code>, with
<p>The second way, which shows good practice, involves building a query, <code>query2</code>, with
a single string literal that includes a wildcard (<code>?</code>). The wildcard
is then given a value by calling <code>setString</code>. This
version is immune to injection attacks, because any special characters
in the result of <code>getCategory</code> are not given any special
treatment.</p>
<sample src="SqlUnescaped.java" />
<sample src="SqlConcatenated.java" />
</example>
<references>

12
java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql → java/ql/src/Security/CWE/CWE-089/SqlConcatenated.ql
View File

@@ -1,7 +1,7 @@
/**
* @name Query built without neutralizing special characters
* @description Building a SQL or Java Persistence query without escaping or otherwise neutralizing any special
* characters is vulnerable to insertion of malicious code.
* @name Query built by concatenation with a possibly-untrusted string
* @description Building a SQL or Java Persistence query by concatenating a possibly-untrusted string
* is vulnerable to insertion of malicious code.
* @kind problem
* @problem.severity error
* @security-severity 8.8
@@ -13,7 +13,7 @@
*/
import java
import semmle.code.java.security.SqlUnescapedLib
import semmle.code.java.security.SqlConcatenatedLib
import semmle.code.java.security.SqlInjectionQuery
class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
@@ -27,7 +27,7 @@ class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
class UncontrolledStringBuilderSourceFlowConfig extends TaintTracking::Configuration {
UncontrolledStringBuilderSourceFlowConfig() {
this = "SqlUnescaped::UncontrolledStringBuilderSourceFlowConfig"
this = "SqlConcatenated::UncontrolledStringBuilderSourceFlowConfig"
}
override predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
@@ -50,5 +50,5 @@ where
)
) and
not queryTaintedBy(query, _, _)
select query, "Query might not neutralize special characters in $@.", uncontrolled,
select query, "Query built by concatenation with $@, which may be untrusted.", uncontrolled,
"this expression"

4
java/ql/src/change-notes/2023-01-03-renamed-sql-unescaped.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The name, description and alert message for the query `java/concatenated-sql-query` have been altered to emphasise that the query flags the use of string concatenation to construct SQL queries, not the lack of appropriate escaping. The query's files have been renamed from `SqlUnescaped.ql` and `SqlUnescapedLib.qll` to `SqlConcatenated.ql` and `SqlConcatenatedLib.qll` respectively; in the unlikely event your custom configuration or queries refer to either of these files by name, those references will need to be adjusted. The query id remains `java/concatenated-sql-query`, so alerts should not be re-raised as a result of this change.

11
java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlConcatenated.expected Normal file
View File

@@ -0,0 +1,11 @@
| Test.java:36:47:36:52 | query1 | Query built by concatenation with $@, which may be untrusted. | Test.java:35:8:35:15 | category | this expression |
| Test.java:42:57:42:62 | query2 | Query built by concatenation with $@, which may be untrusted. | Test.java:41:51:41:52 | id | this expression |
| Test.java:50:62:50:67 | query3 | Query built by concatenation with $@, which may be untrusted. | Test.java:49:8:49:15 | category | this expression |
| Test.java:62:47:62:61 | querySbToString | Query built by concatenation with $@, which may be untrusted. | Test.java:58:19:58:26 | category | this expression |
| Test.java:70:40:70:44 | query | Query built by concatenation with $@, which may be untrusted. | Test.java:69:50:69:54 | price | this expression |
| Test.java:70:40:70:44 | query | Query built by concatenation with $@, which may be untrusted. | Test.java:69:77:69:80 | item | this expression |
| Test.java:78:46:78:50 | query | Query built by concatenation with $@, which may be untrusted. | Test.java:77:50:77:54 | price | this expression |
| Test.java:78:46:78:50 | query | Query built by concatenation with $@, which may be untrusted. | Test.java:77:77:77:80 | item | this expression |
| Test.java:98:47:98:60 | queryFromField | Query built by concatenation with $@, which may be untrusted. | Test.java:97:8:97:19 | categoryName | this expression |
| Test.java:108:47:108:61 | querySbToString | Query built by concatenation with $@, which may be untrusted. | Test.java:104:19:104:30 | categoryName | this expression |
| Test.java:118:47:118:62 | querySb2ToString | Query built by concatenation with $@, which may be untrusted. | Test.java:114:46:114:57 | categoryName | this expression |

1
java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlConcatenated.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE/CWE-089/SqlConcatenated.ql

11
java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlUnescaped.expected
View File

@@ -1,11 +0,0 @@
| Test.java:36:47:36:52 | query1 | Query might not neutralize special characters in $@. | Test.java:35:8:35:15 | category | this expression |
| Test.java:42:57:42:62 | query2 | Query might not neutralize special characters in $@. | Test.java:41:51:41:52 | id | this expression |
| Test.java:50:62:50:67 | query3 | Query might not neutralize special characters in $@. | Test.java:49:8:49:15 | category | this expression |
| Test.java:62:47:62:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:58:19:58:26 | category | this expression |
| Test.java:70:40:70:44 | query | Query might not neutralize special characters in $@. | Test.java:69:50:69:54 | price | this expression |
| Test.java:70:40:70:44 | query | Query might not neutralize special characters in $@. | Test.java:69:77:69:80 | item | this expression |
| Test.java:78:46:78:50 | query | Query might not neutralize special characters in $@. | Test.java:77:50:77:54 | price | this expression |
| Test.java:78:46:78:50 | query | Query might not neutralize special characters in $@. | Test.java:77:77:77:80 | item | this expression |
| Test.java:98:47:98:60 | queryFromField | Query might not neutralize special characters in $@. | Test.java:97:8:97:19 | categoryName | this expression |
| Test.java:108:47:108:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:104:19:104:30 | categoryName | this expression |
| Test.java:118:47:118:62 | querySb2ToString | Query might not neutralize special characters in $@. | Test.java:114:46:114:57 | categoryName | this expression |

1
java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlUnescaped.qlref
View File

@@ -1 +0,0 @@
Security/CWE/CWE-089/SqlUnescaped.ql

2
java/ql/test/query-tests/security/CWE-089/semmle/examples/sbQuery.ql
View File

@@ -1,4 +1,4 @@
import semmle.code.java.security.SqlUnescapedLib
import semmle.code.java.security.SqlConcatenatedLib
from StringBuilderVar sbv, Expr uncontrolled, Method method, int methodLine
where