Skip to content

Commit c8c69aa

Browse files
RasmusWLRasmusWL
authored
Merge pull request github#13561 from amammad/amammad-python-WebAppsConstatntSecretKeys
Python: Flask & Django Constant Secret Key initialization
2 parents 71a36fc + 24f9f13 commit c8c69aa

23 files changed

+696
-0
lines changed

‎python/ql/lib/semmle/python/frameworks/Stdlib.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4372,6 +4372,30 @@ private module StdlibPrivate {
43724372
preservesValue = false
43734373
}
43744374
}
4375+
4376+
/**
4377+
* A flow summary for `os.getenv` / `os.getenvb`
4378+
*
4379+
* See https://devdocs.io/python~3.11/library/os#os.getenv
4380+
*/
4381+
class OsGetEnv extends SummarizedCallable {
4382+
OsGetEnv() { this = "os.getenv" }
4383+
4384+
override DataFlow::CallCfgNode getACall() {
4385+
result = API::moduleImport("os").getMember(["getenv", "getenvb"]).getACall()
4386+
}
4387+
4388+
override DataFlow::ArgumentNode getACallback() {
4389+
result =
4390+
API::moduleImport("os").getMember(["getenv", "getenvb"]).getAValueReachableFromSource()
4391+
}
4392+
4393+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
4394+
input in ["Argument[1]", "Argument[default:]"] and
4395+
output = "ReturnValue" and
4396+
preservesValue = true
4397+
}
4398+
}
43754399
}
43764400

43774401
// ---------------------------------------------------------------------------

‎python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/WebAppConstantSecretKey.qhelp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Flask and Django require a Securely signed key for singing the session cookies. most of the time developers rely on load hardcoded secret keys from a config file or python code. this proves that the way of hardcoded secret can make problems when you forgot to change the constant secret keys.
7+
</p>
8+
</overview>
9+
<recommendation>
10+
<p>
11+
In Flask Consider using a secure random generator with <a href="https://docs.python.org/3/library/secrets.html#secrets.token_hex">Python standard secrets library</a>
12+
</p>
13+
<p>
14+
In Django Consider using a secure random generator with "get_random_secret_key()"" method from "django.core.management.utils".
15+
</p>
16+
</recommendation>
17+
<example>
18+
<p>Safe Django SECRET_KEY</p>
19+
<sample src="examples/example_Django_safe.py" />
20+
<p>Unsafe Django SECRET_KEY Example:</p>
21+
<sample src="examples/example_Django_unsafe.py" />
22+
<p>Safe Flask SECRET_KEY Example:</p>
23+
<sample src="examples/example_Flask_safe.py" />
24+
<sample src="examples/example_Flask_unsafe.py" />
25+
<p>Unsafe Flask SECRET_KEY Example:</p>
26+
<sample src="examples/example_Flask_unsafe2.py" />
27+
<p>config1.py</p>
28+
<sample src="examples/config1.py" />
29+
<p>config2.py</p>
30+
<sample src="examples/config2.py" />
31+
<p>config3.py</p>
32+
<sample src="examples/config3.py" />
33+
<p>__init__.py</p>
34+
<sample src="examples/settings/__init__.py" />
35+
</example>
36+
<references>
37+
<li>
38+
<a href="https://flask.palletsprojects.com/en/2.3.x/config/#SECRET_KEY">Flask Documentation</a>
39+
</li>
40+
<li>
41+
<a href="https://docs.djangoproject.com/en/4.2/ref/settings/#secret-key">Django Documentation</a>
42+
</li>
43+
<li>
44+
<a href="https://flask-jwt-extended.readthedocs.io/en/stable/basic_usage.html#basic-usage">Flask-JWT-Extended Documentation</a>
45+
</li>
46+
<li>
47+
<a href="https://www.horizon3.ai/cve-2023-27524-insecure-default-configuration-in-apache-superset-leads-to-remote-code-execution/">CVE-2023-27524 - Apache Superset had multiple CVEs related to this kind of Vulnerability</a>
48+
</li>
49+
<li>
50+
<a href="https://nvd.nist.gov/vuln/detail/CVE-2020-17526">CVE-2020-17526 - Apache Airflow had multiple CVEs related to this kind of Vulnerability</a>
51+
</li>
52+
<li>
53+
<a href="https://nvd.nist.gov/vuln/detail/CVE-2021-41192">CVE-2021-41192 - Redash was assigning a environment variable with a default value which it was assigning the default secrect if the environment variable does not exists</a>
54+
</li>
55+
56+
</references>
57+
</qhelp>

‎python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/WebAppConstantSecretKey.ql

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* @name Initializing SECRET_KEY of Flask application with Constant value
3+
* @description Initializing SECRET_KEY of Flask application with Constant value
4+
* files can lead to Authentication bypass
5+
* @kind path-problem
6+
* @id py/flask-constant-secret-key
7+
* @problem.severity error
8+
* @security-severity 8.5
9+
* @precision high
10+
* @tags security
11+
* experimental
12+
* external/cwe/cwe-287
13+
*/
14+
15+
import python
16+
import experimental.semmle.python.Concepts
17+
import semmle.python.dataflow.new.DataFlow
18+
import semmle.python.ApiGraphs
19+
import semmle.python.dataflow.new.TaintTracking
20+
import WebAppConstantSecretKeyDjango
21+
import WebAppConstantSecretKeyFlask
22+
import semmle.python.filters.Tests
23+
24+
newtype TFrameWork =
25+
Flask() or
26+
Django()
27+
28+
module WebAppConstantSecretKeyConfig implements DataFlow::StateConfigSig {
29+
class FlowState = TFrameWork;
30+
31+
predicate isSource(DataFlow::Node source, FlowState state) {
32+
state = Flask() and FlaskConstantSecretKeyConfig::isSource(source)
33+
or
34+
state = Django() and DjangoConstantSecretKeyConfig::isSource(source)
35+
}
36+
37+
predicate isBarrier(DataFlow::Node node) {
38+
node.getLocation().getFile().inStdlib()
39+
or
40+
// To reduce FP rate, the following was added
41+
node.getLocation()
42+
.getFile()
43+
.getRelativePath()
44+
.matches(["%test%", "%demo%", "%example%", "%sample%"]) and
45+
// but that also meant all data-flow nodes in query tests were excluded... so we had
46+
// to add this:
47+
not node.getLocation().getFile().getRelativePath().matches("%query-tests/Security/CWE-287%")
48+
}
49+
50+
predicate isSink(DataFlow::Node sink, FlowState state) {
51+
state = Flask() and FlaskConstantSecretKeyConfig::isSink(sink)
52+
or
53+
state = Django() and DjangoConstantSecretKeyConfig::isSink(sink)
54+
}
55+
}
56+
57+
module WebAppConstantSecretKey = TaintTracking::GlobalWithState<WebAppConstantSecretKeyConfig>;
58+
59+
import WebAppConstantSecretKey::PathGraph
60+
61+
from WebAppConstantSecretKey::PathNode source, WebAppConstantSecretKey::PathNode sink
62+
where WebAppConstantSecretKey::flowPath(source, sink)
63+
select sink, source, sink, "The SECRET_KEY config variable is assigned by $@.", source,
64+
" this constant String"

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import python
2+
import experimental.semmle.python.Concepts
3+
import semmle.python.dataflow.new.DataFlow
4+
import semmle.python.ApiGraphs
5+
import semmle.python.dataflow.new.TaintTracking
6+
import WebAppConstantSecretKeySource
7+
8+
module DjangoConstantSecretKeyConfig {
9+
/**
10+
* Sources are Constants that without any Tainting reach the Sinks.
11+
* Also Sources can be the default value of getenv or similar methods
12+
* in a case that no value is assigned to Desired SECRET_KEY environment variable
13+
*/
14+
predicate isSource(DataFlow::Node source) { source instanceof WebAppConstantSecretKeySource }
15+
16+
/**
17+
* Holds if There is a sink like following SECRET_KEY Assignments
18+
* ```python
19+
*from django.conf import settings
20+
*settings.configure(
21+
* SECRET_KEY="constant",
22+
*)
23+
* # or
24+
*settings.SECRET_KEY = "constant"
25+
* ```
26+
*/
27+
predicate isSink(DataFlow::Node sink) {
28+
exists(API::moduleImport("django")) and
29+
(
30+
exists(AssignStmt e | e.getTarget(0).(Name).getId() = ["SECRET_KEY", "SECRET_KEY_FALLBACKS"] |
31+
sink.asExpr() = e.getValue()
32+
)
33+
or
34+
exists(API::Node settings |
35+
settings =
36+
API::moduleImport("django").getMember("conf").getMember(["global_settings", "settings"]) and
37+
sink =
38+
settings
39+
.getMember("configure")
40+
.getKeywordParameter(["SECRET_KEY_FALLBACKS", "SECRET_KEY"])
41+
.asSink()
42+
)
43+
or
44+
exists(DataFlow::AttrWrite attr |
45+
attr.getAttributeName() = ["SECRET_KEY_FALLBACKS", "SECRET_KEY"] and
46+
sink = attr.getValue()
47+
)
48+
) and
49+
exists(sink.getScope().getLocation().getFile().getRelativePath()) and
50+
not sink.getScope().getLocation().getFile().inStdlib()
51+
}
52+
}

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

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import python
2+
import experimental.semmle.python.Concepts
3+
import semmle.python.dataflow.new.DataFlow
4+
import semmle.python.ApiGraphs
5+
import semmle.python.dataflow.new.TaintTracking
6+
import WebAppConstantSecretKeySource
7+
8+
/**
9+
* with using flask-session package, there is no jwt exists in cookies in user side
10+
* ```python
11+
*import os
12+
*from flask import Flask, session
13+
*app = Flask(__name__)
14+
* ```
15+
*/
16+
module FlaskConstantSecretKeyConfig {
17+
/**
18+
* `flask.Flask()`
19+
*/
20+
API::Node flaskInstance() {
21+
result = API::moduleImport("flask").getMember("Flask").getASubclass*()
22+
}
23+
24+
/**
25+
* Sources are Constants that without any Tainting reach the Sinks.
26+
* Also Sources can be the default value of getenv or similar methods
27+
* in a case that no value is assigned to Desired SECRET_KEY environment variable
28+
*/
29+
predicate isSource(DataFlow::Node source) { source instanceof WebAppConstantSecretKeySource }
30+
31+
/**
32+
* Sinks are one of the following kinds, some of them are directly connected to a flask Instance like
33+
* ```python
34+
* app.config['SECRET_KEY'] = 'CHANGEME1'
35+
* app.secret_key = 'CHANGEME2'
36+
* app.config.update(SECRET_KEY="CHANGEME3")
37+
* app.config.from_mapping(SECRET_KEY="CHANGEME4")
38+
* ```
39+
* other Sinks are SECRET_KEY Constants Variables that are defined in separate files or a class in those files like:
40+
* ```python
41+
* app.config.from_pyfile("config.py")
42+
* app.config.from_object('config.Config')
43+
*```
44+
* we find these files with `FromObjectFileName` DataFlow Configuration
45+
* note that "JWT_SECRET_KEY" is same as "SECRET_KEY" but it is belong to popular flask-jwt-extended library
46+
*/
47+
predicate isSink(DataFlow::Node sink) {
48+
(
49+
exists(API::Node n | n = flaskInstance().getReturn() |
50+
sink =
51+
[
52+
n.getMember("config").getSubscript(["SECRET_KEY", "JWT_SECRET_KEY"]).asSink(),
53+
n.getMember("config")
54+
.getMember(["update", "from_mapping"])
55+
.getACall()
56+
.getArgByName(["SECRET_KEY", "JWT_SECRET_KEY"])
57+
]
58+
)
59+
or
60+
exists(DataFlow::AttrWrite attr |
61+
attr.getObject().getALocalSource() = flaskInstance().getACall() and
62+
attr.getAttributeName() = ["secret_key", "jwt_secret_key"] and
63+
sink = attr.getValue()
64+
)
65+
or
66+
exists(SecretKeyAssignStmt e | sink.asExpr() = e.getValue())
67+
) and
68+
exists(sink.getScope().getLocation().getFile().getRelativePath()) and
69+
not sink.getScope().getLocation().getFile().inStdlib()
70+
}
71+
72+
/**
73+
* An Assignments like `SECRET_KEY = ConstantValue`
74+
* and `SECRET_KEY` file must be the Location that is specified in argument of `from_object` or `from_pyfile` methods
75+
*/
76+
class SecretKeyAssignStmt extends AssignStmt {
77+
SecretKeyAssignStmt() {
78+
exists(string configFileName, string fileNamehelper, DataFlow::Node n1, File file |
79+
fileNamehelper = [flaskConfiFileName(n1), flaskConfiFileName2(n1)] and
80+
// because of `from_object` we want first part of `Config.AClassName` which `Config` is a python file name
81+
configFileName = fileNamehelper.splitAt(".") and
82+
file = this.getLocation().getFile()
83+
|
84+
(
85+
if fileNamehelper = "__name__"
86+
then
87+
file.getShortName() = flaskInstance().asSource().getLocation().getFile().getShortName()
88+
else (
89+
fileNamehelper.matches("%.py") and
90+
file.getShortName().matches("%" + configFileName + "%") and
91+
// after spliting, don't look at %py% pattern
92+
configFileName != ".py"
93+
or
94+
// in case of referencing to a directory which then we must look for __init__.py file
95+
not fileNamehelper.matches("%.py") and
96+
file.getRelativePath()
97+
.matches("%" + fileNamehelper.replaceAll(".", "/") + "/__init__.py")
98+
)
99+
) and
100+
this.getTarget(0).(Name).getId() = ["SECRET_KEY", "JWT_SECRET_KEY"]
101+
) and
102+
exists(this.getScope().getLocation().getFile().getRelativePath()) and
103+
not this.getScope().getLocation().getFile().inStdlib()
104+
}
105+
}
106+
107+
/**
108+
* Holds if there is a helper predicate that specify where the Flask `SECRET_KEY` variable location is defined.
109+
* In Flask we have config files that specify the location of `SECRET_KEY` variable initialization
110+
* and the name of these files are determined by
111+
* `app.config.from_pyfile("configFileName.py")`
112+
* or
113+
* `app.config.from_object("configFileName.ClassName")`
114+
*/
115+
string flaskConfiFileName(API::CallNode cn) {
116+
cn =
117+
flaskInstance()
118+
.getReturn()
119+
.getMember("config")
120+
.getMember(["from_object", "from_pyfile"])
121+
.getACall() and
122+
result =
123+
[
124+
cn.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText(),
125+
cn.getParameter(0).asSink().asExpr().(Name).getId()
126+
]
127+
}
128+
129+
string flaskConfiFileName2(API::CallNode cn) {
130+
cn =
131+
API::moduleImport("flask")
132+
.getMember("Flask")
133+
.getASubclass*()
134+
.getASuccessor*()
135+
.getMember("from_object")
136+
.getACall() and
137+
result = cn.getParameter(0).asSink().asExpr().(StrConst).getText()
138+
}
139+
}

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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+
// we should check whether there is a default value or not
10+
exists(API::Node env |
11+
env = API::moduleImport("environ").getMember("Env") and
12+
// has default value
13+
exists(API::Node param | param = env.getKeywordParameter("SECRET_KEY") |
14+
param.asSink().asExpr().getASubExpression*() instanceof StrConst
15+
) and
16+
this = env.getReturn().getReturn().asSource()
17+
)
18+
or
19+
this.asExpr() instanceof StrConst
20+
or
21+
exists(API::CallNode cn |
22+
cn =
23+
[
24+
API::moduleImport("os").getMember("getenv").getACall(),
25+
API::moduleImport("os").getMember("environ").getMember("get").getACall()
26+
] and
27+
cn.getNumArgument() = 2 and
28+
DataFlow::localFlow(any(DataFlow::Node n | n.asExpr() instanceof StrConst), cn.getArg(1)) and
29+
this.asExpr() = cn.asExpr()
30+
)
31+
) and
32+
// 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!
33+
exists(this.getScope().getLocation().getFile().getRelativePath()) and
34+
not this.getScope().getLocation().getFile().inStdlib() and
35+
// special sanitize case for get_random_secret_key and django-environ
36+
not this.getScope().getLocation().getFile().getBaseName() = ["environ.py", "crypto.py"]
37+
}
38+
}

0 commit comments

Comments
 (0)