Skip to content

Commit 7fc5d69

Browse files
committed
Python: Use new taint-tracking query in SQL-injection query.
1 parent b226cb6 commit 7fc5d69

File tree

6 files changed

+25
-10
lines changed

6 files changed

+25
-10
lines changed

python/ql/src/Security/CWE-089/SqlInjection.ql

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,16 @@ import semmle.python.security.injection.Sql
2222
import semmle.python.web.django.Db
2323
import semmle.python.web.django.Model
2424

25+
class SQLInjectionConfiguration extends TaintTracking::Configuration {
2526

26-
from TaintedPathSource src, TaintedPathSink sink
27-
where src.flowsTo(sink)
28-
select sink.getSink(), src, sink, "This SQL query depends on $@.", src.getSource(), "a user-provided value"
27+
SQLInjectionConfiguration() { this = "SQL injection configuration" }
28+
29+
override predicate isSource(TaintTracking::Source source) { source.isSourceOf(any(UntrustedStringKind u)) }
30+
31+
override predicate isSink(TaintTracking::Sink sink) { sink instanceof SqlInjectionSink }
32+
33+
}
34+
35+
from SQLInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink
36+
where config.hasFlowPath(src, sink)
37+
select sink.getNode(), src, sink, "This SQL query depends on $@.", src.getNode(), "a user-provided value"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import python
2+
import semmle.python.security.TaintTracking
3+
4+
abstract class SqlInjectionSink extends TaintSink {}

python/ql/src/semmle/python/security/TaintTracking.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,3 +1668,4 @@ private predicate sequence_call(ControlFlowNode fromnode, CallNode tonode) {
16681668
cls.refersTo(theSetType())
16691669
)
16701670
}
1671+

python/ql/src/semmle/python/security/injection/Sql.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import python
99

1010
import semmle.python.security.TaintTracking
1111
import semmle.python.security.strings.Untrusted
12+
import semmle.python.security.SQL
1213

1314

1415
private StringObject first_part(ControlFlowNode command) {
@@ -48,11 +49,10 @@ abstract class DbCursor extends TaintKind {
4849

4950
}
5051

51-
5252
/** A part of a string that appears to be a SQL command and is thus
5353
* vulnerable to malicious input.
5454
*/
55-
class SimpleSqlStringInjection extends TaintSink {
55+
class SimpleSqlStringInjection extends SqlInjectionSink {
5656

5757
override string toString() { result = "simple SQL string injection" }
5858

@@ -76,7 +76,7 @@ abstract class DbConnectionSource extends TaintSource {
7676
/** A taint sink that is vulnerable to malicious SQL queries.
7777
* The `vuln` in `db.connection.execute(vuln)` and similar.
7878
*/
79-
class DbConnectionExecuteArgument extends TaintSink {
79+
class DbConnectionExecuteArgument extends SqlInjectionSink {
8080

8181
override string toString() { result = "db.connection.execute" }
8282

python/ql/src/semmle/python/web/django/Db.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ ClassObject theDjangoRawSqlClass() {
4646
* allows arbitrary SQL statements to be executed, which is a security risk.
4747
*/
4848

49-
class DjangoRawSqlSink extends TaintSink {
49+
class DjangoRawSqlSink extends SqlInjectionSink {
5050
DjangoRawSqlSink() {
5151
exists(CallNode call |
5252
call = theDjangoRawSqlClass().getACall() and

python/ql/src/semmle/python/web/django/Model.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import python
33
import semmle.python.security.TaintTracking
44
import semmle.python.security.strings.Basic
55
import semmle.python.web.Http
6+
import semmle.python.security.injection.Sql
67

78
/** A django model class */
89
class DjangoModel extends ClassObject {
@@ -68,7 +69,7 @@ class DjangoModelObjects extends TaintSource {
6869
}
6970

7071
/** A write to a field of a django model, which is a vulnerable to external data. */
71-
class DjangoModelFieldWrite extends TaintSink {
72+
class DjangoModelFieldWrite extends SqlInjectionSink {
7273

7374
DjangoModelFieldWrite() {
7475
exists(AttrNode attr, DjangoModel model |
@@ -111,7 +112,7 @@ class DjangoModelDirectObjectReference extends TaintSink {
111112
* to be sent to the database, which is a security risk.
112113
*/
113114

114-
class DjangoModelRawCall extends TaintSink {
115+
class DjangoModelRawCall extends SqlInjectionSink {
115116

116117
DjangoModelRawCall() {
117118
exists(CallNode raw_call, ControlFlowNode queryset |
@@ -136,7 +137,7 @@ class DjangoModelRawCall extends TaintSink {
136137
*/
137138

138139

139-
class DjangoModelExtraCall extends TaintSink {
140+
class DjangoModelExtraCall extends SqlInjectionSink {
140141

141142
DjangoModelExtraCall() {
142143
exists(CallNode extra_call, ControlFlowNode queryset |

0 commit comments

Comments
 (0)