Skip to content

Commit 13fa08a

Browse files
committed
Python: Move source modeling to shared file
1 parent aa8ed91 commit 13fa08a

File tree

3 files changed

+69
-116
lines changed

3 files changed

+69
-116
lines changed

python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/WebAppConstantSecretKeyDjango.qll

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3,71 +3,15 @@ import experimental.semmle.python.Concepts
33
import semmle.python.dataflow.new.DataFlow
44
import semmle.python.ApiGraphs
55
import semmle.python.dataflow.new.TaintTracking
6+
import WebAppConstantSecretKeySource
67

78
module DjangoConstantSecretKeyConfig {
89
/**
910
* Sources are Constants that without any Tainting reach the Sinks.
1011
* Also Sources can be the default value of getenv or similar methods
1112
* in a case that no value is assigned to Desired SECRET_KEY environment variable
1213
*/
13-
predicate isSource(DataFlow::Node source) {
14-
(
15-
// because Env return an Exeption if there isan't any value (instead of none) we should check whether
16-
// there is a default value of there is a config file which mostly these config files have a default value
17-
exists(API::Node env | env = API::moduleImport("environ").getMember("Env") |
18-
(
19-
// has default value
20-
exists(env.getKeywordParameter("SECRET_KEY").asSource())
21-
or
22-
// get value from a config file which is not best security practice
23-
exists(env.getReturn().getMember("read_env"))
24-
) and
25-
source = env.getReturn().getReturn().asSource()
26-
)
27-
or
28-
source.asExpr().isConstant()
29-
or
30-
exists(API::Node cn |
31-
cn =
32-
[
33-
API::moduleImport("configparser")
34-
.getMember(["ConfigParser", "RawConfigParser"])
35-
.getReturn(),
36-
// legacy API https://docs.python.org/3/library/configparser.html#legacy-api-examples
37-
API::moduleImport("configparser")
38-
.getMember(["ConfigParser", "RawConfigParser"])
39-
.getReturn()
40-
.getMember("get")
41-
.getReturn()
42-
] and
43-
source = cn.asSource()
44-
)
45-
or
46-
exists(API::CallNode cn |
47-
cn =
48-
[
49-
API::moduleImport("os").getMember("getenv").getACall(),
50-
API::moduleImport("os").getMember("environ").getMember("get").getACall()
51-
] and
52-
(
53-
// this can be ideal if we assume that best security practice is that
54-
// we don't get SECRET_KEY from env and we always assign a secure generated random string to it
55-
cn.getNumArgument() = 1
56-
or
57-
cn.getNumArgument() = 2 and
58-
DataFlow::localFlow(any(DataFlow::Node n | n.asExpr().isConstant()), cn.getArg(1))
59-
) and
60-
source.asExpr() = cn.asExpr()
61-
)
62-
or
63-
source = API::moduleImport("os").getMember("environ").getASubscript().asSource()
64-
) and
65-
// followings will sanitize the get_random_secret_key of django.core.management.utils and similar random generators which we have their source code and some of them can be tracking by taint tracking because they are initilized by a constant!
66-
exists(source.getScope().getLocation().getFile().getRelativePath()) and
67-
not source.getScope().getLocation().getFile().inStdlib() and
68-
// special sanitize case for get_random_secret_key and django-environ
69-
not source.getScope().getLocation().getFile().getBaseName() = ["environ.py", "crypto.py"]
70-
}
14+
predicate isSource(DataFlow::Node source) { source instanceof WebAppConstantSecretKeySource }
7115

7216
/**
7317
* A sink like following SECRET_KEY Assignments

python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/WebAppConstantSecretKeyFlask.qll

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import experimental.semmle.python.Concepts
33
import semmle.python.dataflow.new.DataFlow
44
import semmle.python.ApiGraphs
55
import semmle.python.dataflow.new.TaintTracking
6+
import WebAppConstantSecretKeySource
67

78
/**
89
* with using flask-session package, there is no jwt exists in cookies in user side
@@ -25,64 +26,7 @@ module FlaskConstantSecretKeyConfig {
2526
* Also Sources can be the default value of getenv or similar methods
2627
* in a case that no value is assigned to Desired SECRET_KEY environment variable
2728
*/
28-
predicate isSource(DataFlow::Node source) {
29-
(
30-
// because Env return an Exeption if there isan't any value (instead of none) we should check whether
31-
// there is a default value of there is a config file which mostly these config files have a default value
32-
exists(API::Node env | env = API::moduleImport("environ").getMember("Env") |
33-
(
34-
// has default value
35-
exists(env.getKeywordParameter("SECRET_KEY").asSource())
36-
or
37-
// get value from a config file which is not best security practice
38-
exists(env.getReturn().getMember("read_env"))
39-
) and
40-
source = env.getReturn().getReturn().asSource()
41-
)
42-
or
43-
source.asExpr().isConstant()
44-
or
45-
exists(API::Node cn |
46-
cn =
47-
[
48-
API::moduleImport("configparser")
49-
.getMember(["ConfigParser", "RawConfigParser"])
50-
.getReturn(),
51-
// legacy API https://docs.python.org/3/library/configparser.html#legacy-api-examples
52-
API::moduleImport("configparser")
53-
.getMember(["ConfigParser", "RawConfigParser"])
54-
.getReturn()
55-
.getMember("get")
56-
.getReturn()
57-
] and
58-
source = cn.asSource()
59-
)
60-
or
61-
exists(API::CallNode cn |
62-
cn =
63-
[
64-
API::moduleImport("os").getMember("getenv").getACall(),
65-
API::moduleImport("os").getMember("environ").getMember("get").getACall()
66-
] and
67-
(
68-
// this can be ideal if we assume that best security practice is that
69-
// we don't get SECRET_KEY from env and we always assign a secure generated random string to it
70-
cn.getNumArgument() = 1
71-
or
72-
cn.getNumArgument() = 2 and
73-
DataFlow::localFlow(any(DataFlow::Node n | n.asExpr().isConstant()), cn.getArg(1))
74-
) and
75-
source.asExpr() = cn.asExpr()
76-
)
77-
or
78-
source.asExpr() =
79-
API::moduleImport("os").getMember("environ").getASubscript().asSource().asExpr()
80-
) and
81-
exists(source.getScope().getLocation().getFile().getRelativePath()) and
82-
not source.getScope().getLocation().getFile().inStdlib() and
83-
// special sanitize case for django-environ
84-
not source.getScope().getLocation().getFile().getBaseName() = "crypto.py"
85-
}
29+
predicate isSource(DataFlow::Node source) { source instanceof WebAppConstantSecretKeySource }
8630

8731
/**
8832
* Sinks are one of the following kinds, some of them are directly connected to a flask Instance like
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import python
2+
import semmle.python.dataflow.new.DataFlow
3+
import semmle.python.ApiGraphs
4+
5+
/** A bad source for secret key in web app configurations. */
6+
class WebAppConstantSecretKeySource extends DataFlow::Node {
7+
WebAppConstantSecretKeySource() {
8+
(
9+
// because Env return an Exeption if there isan't any value (instead of none) we should check whether
10+
// there is a default value of there is a config file which mostly these config files have a default value
11+
exists(API::Node env | env = API::moduleImport("environ").getMember("Env") |
12+
(
13+
// has default value
14+
exists(env.getKeywordParameter("SECRET_KEY").asSource())
15+
or
16+
// get value from a config file which is not best security practice
17+
exists(env.getReturn().getMember("read_env"))
18+
) and
19+
this = env.getReturn().getReturn().asSource()
20+
)
21+
or
22+
this.asExpr().isConstant()
23+
or
24+
exists(API::Node cn |
25+
cn =
26+
[
27+
API::moduleImport("configparser")
28+
.getMember(["ConfigParser", "RawConfigParser"])
29+
.getReturn(),
30+
// legacy API https://docs.python.org/3/library/configparser.html#legacy-api-examples
31+
API::moduleImport("configparser")
32+
.getMember(["ConfigParser", "RawConfigParser"])
33+
.getReturn()
34+
.getMember("get")
35+
.getReturn()
36+
] and
37+
this = cn.asSource()
38+
)
39+
or
40+
exists(API::CallNode cn |
41+
cn =
42+
[
43+
API::moduleImport("os").getMember("getenv").getACall(),
44+
API::moduleImport("os").getMember("environ").getMember("get").getACall()
45+
] and
46+
(
47+
// this can be ideal if we assume that best security practice is that
48+
// we don't get SECRET_KEY from env and we always assign a secure generated random string to it
49+
cn.getNumArgument() = 1
50+
or
51+
cn.getNumArgument() = 2 and
52+
DataFlow::localFlow(any(DataFlow::Node n | n.asExpr().isConstant()), cn.getArg(1))
53+
) and
54+
this.asExpr() = cn.asExpr()
55+
)
56+
or
57+
this = API::moduleImport("os").getMember("environ").getASubscript().asSource()
58+
) and
59+
// followings will sanitize the get_random_secret_key of django.core.management.utils and similar random generators which we have their source code and some of them can be tracking by taint tracking because they are initilized by a constant!
60+
exists(this.getScope().getLocation().getFile().getRelativePath()) and
61+
not this.getScope().getLocation().getFile().inStdlib() and
62+
// special sanitize case for get_random_secret_key and django-environ
63+
not this.getScope().getLocation().getFile().getBaseName() = ["environ.py", "crypto.py"]
64+
}
65+
}

0 commit comments

Comments
 (0)