This commit is contained in:
amammad
2023-06-21 07:21:38 +10:00
parent 5259a6ecfc
commit e3e0307db7
11 changed files with 447 additions and 0 deletions

View File

@@ -0,0 +1,45 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
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.
</p>
</overview>
<recommendation>
<p>
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>
</p>
<p>
In Django Consider using a secure random generator with "get_random_secret_key()"" method from "django.core.management.utils".
</p>
</recommendation>
<example>
<sample src="example_bad.py" />
<sample src="example_good.py" />
</example>
<references>
<li>
<a href="https://flask.palletsprojects.com/en/2.3.x/config/#SECRET_KEY">Flask Documentation</a>
</li>
<li>
<a href="https://docs.djangoproject.com/en/4.2/ref/settings/#secret-key">Django Documentation</a>
</li>
<li>
<a href="https://flask-jwt-extended.readthedocs.io/en/stable/basic_usage.html#basic-usage">Flask-JWT-Extended Documentation</a>
</li>
<li>
<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>
</li>
<li>
<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>
</li>
<li>
<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>
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,219 @@
/**
* @name Initializing SECRET_KEY of Flask application with Constant value
* @description Initializing SECRET_KEY of Flask application with Constant value
* files can lead to Authentication bypass
* @kind path-problem
* @id py/ConstantSecretKey
* @problem.severity error
* @security-severity 8.5
* @precision high
* @tags security
* experimental
* external/cwe/cwe-287
*/
import python
import experimental.semmle.python.Concepts
import semmle.python.dataflow.new.DataFlow
import semmle.python.ApiGraphs
import semmle.python.dataflow.new.TaintTracking
/**
* `flask.Flask()`
*/
API::Node flaskInstance() { result = API::moduleImport("flask").getMember("Flask").getASubclass*() }
/**
* with using flask-session package, there is no jwt exists in cookies in user side
* ```python
*import os
*from flask import Flask, session
*app = Flask(__name__)
* ```
*/
module FlaskConstantSecretKeyConfig implements DataFlow::ConfigSig {
/**
* Sources are Constants that without any Tainting reach the Sinks.
* Also Sources can be the default value of getenv or similar methods
* in a case that no value is assigned to Desired SECRET_KEY environment variable
*/
predicate isSource(DataFlow::Node source) {
(
source.asExpr().isConstant()
or
exists(API::Node cn |
cn =
[
API::moduleImport("configparser")
.getMember(["ConfigParser", "RawConfigParser"])
.getReturn(),
// legacy API https://docs.python.org/3/library/configparser.html#legacy-api-examples
API::moduleImport("configparser")
.getMember(["ConfigParser", "RawConfigParser"])
.getReturn()
.getMember("get")
.getReturn()
] and
source = cn.asSource()
)
or
exists(API::CallNode cn |
cn =
[
API::moduleImport("os").getMember("getenv").getACall(),
API::moduleImport("os").getMember("environ").getMember("get").getACall()
] and
(
// this can be ideal if we assume that best security practice is that
// we don't get SECRET_KEY from env and we always assign a secure generated random string to it
cn.getNumArgument() = 1
or
cn.getNumArgument() = 2 and
DataFlow::localFlow(any(DataFlow::Node n | n.asExpr().isConstant()), cn.getArg(1))
) and
source.asExpr() = cn.asExpr()
)
or
exists(DataFlow::LocalSourceNode lsn |
lsn = API::moduleImport("os").getMember("environ").getASubscript().asSource() and
source.asExpr() = lsn.asExpr()
)
) and
not source.getScope().getLocation().getFile().inStdlib()
}
/**
* Sinks are one of the following kinds, some of them are directly connected to a flask Instance like
* ```python
* app.config['SECRET_KEY'] = 'CHANGEME1'
* app.secret_key = 'CHANGEME2'
* app.config.update(SECRET_KEY="CHANGEME3")
* app.config.from_mapping(SECRET_KEY="CHANGEME4")
* ```
* other Sinks are SECRET_KEY Constants Variables that are defined in seperate files or a class in those files like:
* ```python
* app.config.from_pyfile("config.py")
* app.config.from_object('config.Config')
*```
* we find these files with `FromObjectFileName` DataFlow Configuration
* note that "JWT_SECRET_KEY" is same as "SECRET_KEY" but it is belong to popular flask-jwt-extended library
*/
predicate isSink(DataFlow::Node sink) {
(
exists(API::Node n |
n = flaskInstance() and
flask_sessionSanitizer(n.getReturn().asSource())
|
sink =
[
n.getReturn().getAMember().getSubscript(["SECRET_KEY", "JWT_SECRET_KEY"]).asSink(),
n.getReturn().getMember(["SECRET_KEY", "JWT_SECRET_KEY"]).asSink(),
n.getReturn()
.getMember("config")
.getMember(["update", "from_mapping"])
.getACall()
.getArgByName(["SECRET_KEY", "JWT_SECRET_KEY"])
]
)
or
// this query checks for Django SecretKey too
if exists(API::moduleImport("django"))
then
exists(AssignStmt e | e.getTarget(0).toString() = "SECRET_KEY" |
sink.asExpr() = e.getValue()
// and sanitizer(e.getTarget(0))
)
else
exists(SecretKeyAssignStmt e |
sink.asExpr() = e.getValue()
// | sanitizer(e.getTarget(0))
)
) and
not sink.getScope().getLocation().getFile().inStdlib()
}
}
// using flask_session library is safe
predicate flask_sessionSanitizer(DataFlow::Node source) {
not DataFlow::localFlow(source,
API::moduleImport("flask_session").getMember("Session").getACall().getArg(0))
}
// *it seems that sanitizer have a lot of performance issues*
// for case check whether SECRECT_KEY is empty or not
predicate sanitizer(Expr sourceExpr) {
exists(DataFlow::Node source, DataFlow::Node sink, If i |
source.asExpr() = sourceExpr and
DataFlow::localFlow(source, sink)
|
not i.getASubExpression().getAChildNode*().(Compare) = sink.asExpr() and
not sink.getScope().getLocation().getFile().inStdlib() and
not source.getScope().getLocation().getFile().inStdlib() and
not i.getScope().getLocation().getFile().inStdlib()
)
}
/**
* Assignments like `SECRET_KEY = ConstantValue`
* which ConstantValue will be found by another DataFlow Configuration
* and `SECRET_KEY` location must be a argument of `from_object` or `from_pyfile` methods
* the argument/location value will be found by another Taint Tracking Configuration.
*/
class SecretKeyAssignStmt extends AssignStmt {
SecretKeyAssignStmt() {
exists(
string configFileName, string fileNamehelper, DataFlow::Node n1, FromObjectFileName config
|
config.hasFlow(n1, _) and
n1.asExpr().isConstant() and
fileNamehelper = n1.asExpr().(StrConst).getS() and
// because of `from_object` we want first part of `Config.AClassName` which `Config` is a python file name
configFileName = fileNamehelper.splitAt(".") and
// after spliting, don't look at %py% pattern
configFileName != "py"
|
this.getLocation().getFile().getShortName().matches("%" + configFileName + "%") and
this.getTarget(0).toString() = ["SECRET_KEY", "JWT_SECRET_KEY"]
) and
not this.getScope().getLocation().getFile().inStdlib()
}
}
/**
* we have some file name that telling us the SECRET_KEY location
* which have determined by these two methods
* `app.config.from_pyfile("configFileName.py")` or `app.config.from_object("configFileName.ClassName")`
* this is a helper configuration that help us skip the SECRET_KEY variables that are not related to Flask.
*/
class FromObjectFileName extends TaintTracking::Configuration {
FromObjectFileName() { this = "FromObjectFileName" }
override predicate isSource(DataFlow::Node source) {
source.asExpr().isConstant() and
not source.getScope().getLocation().getFile().inStdlib()
}
override predicate isSink(DataFlow::Node sink) {
exists(API::Node n |
n = flaskInstance() and
flask_sessionSanitizer(n.getReturn().asSource())
|
sink =
n.getReturn()
.getMember("config")
.getMember(["from_object", "from_pyfile"])
.getACall()
.getArg(0)
) and
not sink.getScope().getLocation().getFile().inStdlib()
}
}
module FlaskConstantSecretKey = TaintTracking::Global<FlaskConstantSecretKeyConfig>;
import FlaskConstantSecretKey::PathGraph
from FlaskConstantSecretKey::PathNode source, FlaskConstantSecretKey::PathNode sink
where FlaskConstantSecretKey::flowPath(source, sink)
select sink, source, sink, "The SECRET_KEY config variable has assigned by $@.", source,
" this constant String"

View File

@@ -0,0 +1,18 @@
from flask import Flask, session
app = Flask(__name__)
aConstant = 'CHANGEME1'
app.config['SECRET_KEY'] = aConstant
@app.route('/')
def DEB_EX():
if 'logged_in' not in session:
session['logged_in'] = False
if session['logged_in']:
return app.secret_key
else:
return app.secret_key, 403
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,19 @@
from flask import Flask, session
from secrets import token_hex
app = Flask(__name__)
app.config['SECRET_KEY'] = token_hex(64)
@app.route('/')
def DEB_EX():
if 'logged_in' not in session:
session['logged_in'] = False
if session['logged_in']:
return app.secret_key
else:
return app.secret_key, 403
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,52 @@
edges
| app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:5:28:5:36 | ControlFlowNode for aConstant |
| app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:7:30:7:38 | ControlFlowNode for aConstant |
| app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:8:36:8:44 | ControlFlowNode for aConstant |
| config.py:7:1:7:9 | GSSA Variable aConstant | config.py:12:18:12:26 | ControlFlowNode for aConstant |
| config.py:7:1:7:9 | GSSA Variable aConstant | config.py:12:18:12:26 | ControlFlowNode for aConstant |
| config.py:7:1:7:9 | GSSA Variable aConstant | config.py:17:38:17:46 | ControlFlowNode for aConstant |
| config.py:7:1:7:9 | GSSA Variable aConstant | config.py:18:43:18:51 | ControlFlowNode for aConstant |
| config.py:7:13:7:23 | ControlFlowNode for Str | config.py:7:1:7:9 | GSSA Variable aConstant |
| config.py:12:18:12:26 | ControlFlowNode for aConstant | config.py:17:38:17:46 | ControlFlowNode for aConstant |
| config.py:12:18:12:26 | ControlFlowNode for aConstant | config.py:18:43:18:51 | ControlFlowNode for aConstant |
| config.py:17:38:17:46 | ControlFlowNode for aConstant | config.py:17:18:17:47 | ControlFlowNode for Attribute() |
| config.py:17:38:17:46 | ControlFlowNode for aConstant | config.py:18:43:18:51 | ControlFlowNode for aConstant |
| config.py:17:38:17:46 | ControlFlowNode for aConstant | file:///usr/lib/python3.10/os.py:771:17:771:23 | ControlFlowNode for default |
| config.py:18:43:18:51 | ControlFlowNode for aConstant | config.py:18:18:18:52 | ControlFlowNode for Attribute() |
| file:///usr/lib/python3.10/os.py:771:17:771:23 | ControlFlowNode for default | file:///usr/lib/python3.10/os.py:775:29:775:35 | ControlFlowNode for default |
| file:///usr/lib/python3.10/os.py:775:29:775:35 | ControlFlowNode for default | file:///usr/lib/python3.10/os.py:775:12:775:36 | ControlFlowNode for Attribute() |
nodes
| app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
| app_unsafe.py:5:28:5:36 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| app_unsafe.py:7:30:7:38 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| app_unsafe.py:8:36:8:44 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| config2.py:5:14:5:24 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
| config.py:7:1:7:9 | GSSA Variable aConstant | semmle.label | GSSA Variable aConstant |
| config.py:7:13:7:23 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
| config.py:11:18:11:38 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| config.py:12:18:12:26 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| config.py:12:18:12:26 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| config.py:13:18:13:36 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| config.py:14:18:14:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| config.py:17:18:17:47 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| config.py:17:38:17:46 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| config.py:18:18:18:52 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| config.py:18:43:18:51 | ControlFlowNode for aConstant | semmle.label | ControlFlowNode for aConstant |
| config.py:19:18:19:37 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
| file:///usr/lib/python3.10/os.py:771:17:771:23 | ControlFlowNode for default | semmle.label | ControlFlowNode for default |
| file:///usr/lib/python3.10/os.py:775:12:775:36 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| file:///usr/lib/python3.10/os.py:775:29:775:35 | ControlFlowNode for default | semmle.label | ControlFlowNode for default |
subpaths
| config.py:17:38:17:46 | ControlFlowNode for aConstant | file:///usr/lib/python3.10/os.py:771:17:771:23 | ControlFlowNode for default | file:///usr/lib/python3.10/os.py:775:12:775:36 | ControlFlowNode for Attribute() | config.py:17:18:17:47 | ControlFlowNode for Attribute() |
#select
| app_unsafe.py:5:28:5:36 | ControlFlowNode for aConstant | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:5:28:5:36 | ControlFlowNode for aConstant | The SECRET_KEY config variable has assigned by $@. | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | this constant String |
| app_unsafe.py:7:30:7:38 | ControlFlowNode for aConstant | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:7:30:7:38 | ControlFlowNode for aConstant | The SECRET_KEY config variable has assigned by $@. | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | this constant String |
| app_unsafe.py:8:36:8:44 | ControlFlowNode for aConstant | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | app_unsafe.py:8:36:8:44 | ControlFlowNode for aConstant | The SECRET_KEY config variable has assigned by $@. | app_unsafe.py:4:13:4:23 | ControlFlowNode for Str | this constant String |
| config2.py:5:14:5:24 | ControlFlowNode for Str | config2.py:5:14:5:24 | ControlFlowNode for Str | config2.py:5:14:5:24 | ControlFlowNode for Str | The SECRET_KEY config variable has assigned by $@. | config2.py:5:14:5:24 | ControlFlowNode for Str | this constant String |
| config.py:11:18:11:38 | ControlFlowNode for Attribute() | config.py:11:18:11:38 | ControlFlowNode for Attribute() | config.py:11:18:11:38 | ControlFlowNode for Attribute() | The SECRET_KEY config variable has assigned by $@. | config.py:11:18:11:38 | ControlFlowNode for Attribute() | this constant String |
| config.py:12:18:12:26 | ControlFlowNode for aConstant | config.py:7:13:7:23 | ControlFlowNode for Str | config.py:12:18:12:26 | ControlFlowNode for aConstant | The SECRET_KEY config variable has assigned by $@. | config.py:7:13:7:23 | ControlFlowNode for Str | this constant String |
| config.py:13:18:13:36 | ControlFlowNode for Attribute() | config.py:13:18:13:36 | ControlFlowNode for Attribute() | config.py:13:18:13:36 | ControlFlowNode for Attribute() | The SECRET_KEY config variable has assigned by $@. | config.py:13:18:13:36 | ControlFlowNode for Attribute() | this constant String |
| config.py:14:18:14:41 | ControlFlowNode for Attribute() | config.py:14:18:14:41 | ControlFlowNode for Attribute() | config.py:14:18:14:41 | ControlFlowNode for Attribute() | The SECRET_KEY config variable has assigned by $@. | config.py:14:18:14:41 | ControlFlowNode for Attribute() | this constant String |
| config.py:17:18:17:47 | ControlFlowNode for Attribute() | config.py:7:13:7:23 | ControlFlowNode for Str | config.py:17:18:17:47 | ControlFlowNode for Attribute() | The SECRET_KEY config variable has assigned by $@. | config.py:7:13:7:23 | ControlFlowNode for Str | this constant String |
| config.py:18:18:18:52 | ControlFlowNode for Attribute() | config.py:7:13:7:23 | ControlFlowNode for Str | config.py:18:18:18:52 | ControlFlowNode for Attribute() | The SECRET_KEY config variable has assigned by $@. | config.py:7:13:7:23 | ControlFlowNode for Str | this constant String |
| config.py:19:18:19:37 | ControlFlowNode for Subscript | config.py:19:18:19:37 | ControlFlowNode for Subscript | config.py:19:18:19:37 | ControlFlowNode for Subscript | The SECRET_KEY config variable has assigned by $@. | config.py:19:18:19:37 | ControlFlowNode for Subscript | this constant String |

View File

@@ -0,0 +1 @@
experimental/Security/CWE-287-ConstantSecretKey/ConstantSecretKey.ql

View File

@@ -0,0 +1,22 @@
from flask import Flask, session
from flask_session import Session
app = Flask(__name__)
app.config['SECRET_KEY'] = 'CHANGEME'
Session(app)
@app.route('/')
def index():
if 'logged_in' not in session:
session['logged_in'] = False
if session['logged_in']:
return '<h1>You are logged in!</h1>'
else:
return '<h1>Access Denied</h1>', 403
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,23 @@
from flask import Flask, session
from secrets import token_hex
app = Flask(__name__)
SECRET_KEY = 'CHANGEME'
if not SECRET_KEY:
SECRET_KEY = token_hex(16)
@app.route('/')
def index():
if 'logged_in' not in session:
session['logged_in'] = False
if session['logged_in']:
return '<h1>You are logged in!</h1>'
else:
return '<h1>Access Denied</h1>', 403
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,24 @@
from flask import Flask, session
app = Flask(__name__)
aConstant = 'CHANGEME1'
app.config['SECRET_KEY'] = aConstant
app.secret_key = aConstant
app.config.update(SECRET_KEY=aConstant)
app.config.from_mapping(SECRET_KEY=aConstant)
app.config.from_pyfile("config.py")
app.config.from_object('config.Config')
@app.route('/')
def DEB_EX():
if 'logged_in' not in session:
session['logged_in'] = False
if session['logged_in']:
return app.secret_key
else:
return app.secret_key, 403
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,19 @@
"""Flask App configuration."""
from os import environ
import os
import random
FLASK_DEBUG = True
aConstant = 'CHANGEME2'
class Config:
SECRET_KEY = environ.get("envKey")
SECRET_KEY = aConstant
SECRET_KEY = os.getenv('envKey')
SECRET_KEY = os.environ.get('envKey')
SECRET_KEY = os.environ.get('envKey', random.randint)
SECRET_KEY = os.getenv('envKey', random.randint)
SECRET_KEY = os.getenv('envKey', aConstant)
SECRET_KEY = os.environ.get('envKey', aConstant)
SECRET_KEY = os.environ['envKey']

View File

@@ -0,0 +1,5 @@
"""Flask App configuration."""
# General Config
FLASK_DEBUG = True
SECRET_KEY = "CHANGEME5"