Skip to content

Commit 45c732a

Browse files
committed
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.
1 parent 06ea249 commit 45c732a

File tree

10 files changed

+24
-24
lines changed

10 files changed

+24
-24
lines changed

java/ql/lib/semmle/code/java/security/SqlUnescapedLib.qll renamed to java/ql/lib/semmle/code/java/security/SqlConcatenatedLib.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** Definitions used by `SqlUnescaped.ql`. */
1+
/** Definitions used by `SqlConcatenated.ql`. */
22

33
import semmle.code.java.security.ControlledString
44
import semmle.code.java.dataflow.TaintTracking

java/ql/lib/semmle/code/java/security/SqlInjectionQuery.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class QueryInjectionFlowConfig extends TaintTracking::Configuration {
3333

3434
/**
3535
* Implementation of `SqlTainted.ql`. This is extracted to a QLL so that it
36-
* can be excluded from `SqlUnescaped.ql` to avoid overlapping results.
36+
* can be excluded from `SqlConcatenated.ql` to avoid overlapping results.
3737
*/
3838
predicate queryTaintedBy(
3939
QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink

java/ql/src/Security/CWE/CWE-089/SqlUnescaped.qhelp renamed to java/ql/src/Security/CWE/CWE-089/SqlConcatenated.qhelp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,18 @@ it enough to cause the SQL query to fail to run.</p>
2121
<p>In the following example, the code runs a simple SQL query in two different ways.</p>
2222

2323
<p>The first way involves building a query, <code>query1</code>, by concatenating the
24-
result of <code>getCategory</code> with some string literals. The result of
24+
result of <code>getCategory</code> with some string literals. The result of
2525
<code>getCategory</code> can include special characters, or
2626
it might be refactored later so that it may return something that contains special characters.</p>
2727

28-
<p>The second way, which shows good practice, involves building a query, <code>query2</code>, with
28+
<p>The second way, which shows good practice, involves building a query, <code>query2</code>, with
2929
a single string literal that includes a wildcard (<code>?</code>). The wildcard
3030
is then given a value by calling <code>setString</code>. This
3131
version is immune to injection attacks, because any special characters
3232
in the result of <code>getCategory</code> are not given any special
3333
treatment.</p>
3434

35-
<sample src="SqlUnescaped.java" />
35+
<sample src="SqlConcatenated.java" />
3636

3737
</example>
3838
<references>

java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql renamed to java/ql/src/Security/CWE/CWE-089/SqlConcatenated.ql

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
2-
* @name Query built without neutralizing special characters
3-
* @description Building a SQL or Java Persistence query without escaping or otherwise neutralizing any special
4-
* characters is vulnerable to insertion of malicious code.
2+
* @name Query built by concatenation with a possibly-untrusted string
3+
* @description Building a SQL or Java Persistence query by concatenating a possibly-untrusted string
4+
* is vulnerable to insertion of malicious code.
55
* @kind problem
66
* @problem.severity error
77
* @security-severity 8.8
@@ -13,7 +13,7 @@
1313
*/
1414

1515
import java
16-
import semmle.code.java.security.SqlUnescapedLib
16+
import semmle.code.java.security.SqlConcatenatedLib
1717
import semmle.code.java.security.SqlInjectionQuery
1818

1919
class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
@@ -27,7 +27,7 @@ class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
2727

2828
class UncontrolledStringBuilderSourceFlowConfig extends TaintTracking::Configuration {
2929
UncontrolledStringBuilderSourceFlowConfig() {
30-
this = "SqlUnescaped::UncontrolledStringBuilderSourceFlowConfig"
30+
this = "SqlConcatenated::UncontrolledStringBuilderSourceFlowConfig"
3131
}
3232

3333
override predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
@@ -50,5 +50,5 @@ where
5050
)
5151
) and
5252
not queryTaintedBy(query, _, _)
53-
select query, "Query might not neutralize special characters in $@.", uncontrolled,
53+
select query, "Query built by concatenation with $@, which may be untrusted.", uncontrolled,
5454
"this expression"
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
| 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 |
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |
6+
| 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 |
7+
| 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 |
8+
| 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 |
9+
| 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 |
10+
| 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 |
11+
| 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 |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-089/SqlConcatenated.ql

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

Lines changed: 0 additions & 11 deletions
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import semmle.code.java.security.SqlUnescapedLib
1+
import semmle.code.java.security.SqlConcatenatedLib
22

33
from StringBuilderVar sbv, Expr uncontrolled, Method method, int methodLine
44
where

0 commit comments

Comments
 (0)