mirror of
https://github.com/github/codeql.git
synced 2026-05-03 12:45:27 +02:00
Java: improve naming and description of SqlUnescaped.ql
Since the main thing it's objecting to is concatenation not lack of escaping (in particular it doesn't look for escaping sanitizers), rename and re-describe it accordingly.
This commit is contained in:
@@ -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
|
||||
@@ -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
|
||||
|
||||
@@ -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>
|
||||
@@ -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"
|
||||
@@ -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 |
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-089/SqlConcatenated.ql
|
||||
@@ -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 +0,0 @@
|
||||
Security/CWE/CWE-089/SqlUnescaped.ql
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user