hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

upgrade query to detect redash CVE too

Browse Source
This commit is contained in:
amammad
2023-06-30 22:14:50 +10:00
parent 7a17b99c17
commit 816799c4ba
436 changed files with 13346 additions and 1089 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
python/ql/src/CHANGELOG.md
View File

@@ -1,3 +1,9 @@
## 0.7.3
### Bug Fixes
* The display name (`@name`) of the `py/unsafe-deserialization` query has been updated in favor of consistency with other languages.
## 0.7.2
No user-facing changes.

5
python/ql/src/change-notes/released/0.7.3.md Normal file
View File

@@ -0,0 +1,5 @@
## 0.7.3
### Bug Fixes
* The display name (`@name`) of the `py/unsafe-deserialization` query has been updated in favor of consistency with other languages.

2
python/ql/src/codeql-pack.release.yml
View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.7.2
lastReleaseVersion: 0.7.3

49
python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/WebAppConstantSecretKey.qhelp Normal file
View File

@@ -0,0 +1,49 @@
<!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="examples/example_Django_safe.py" />
<sample src="examples/example_Django_snsafe.py" />
<sample src="examples/example_Flask_safe.py" />
<sample src="examples/example_Flask_unsafe.py" />
<sample src="examples/example_Flask_unsafe2.py" />
<sample src="examples/config1.py" />
<sample src="examples/config2.py" />
<sample src="examples/config3.py" />
<sample src="examples/settings/__init__.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>

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

@@ -12,6 +12,19 @@ module DjangoConstantSecretKeyConfig {
*/
predicate isSource(DataFlow::Node source) {
(
// because Env return an Exeption if there isan't any value (instead of none) we should check whether
// there is a default value of there is a config file which mostly these config files have a default value
exists(API::Node env | env = API::moduleImport("environ").getMember("Env") |
(
// has default value
exists(env.getKeywordParameter("SECRET_KEY").asSource())
or
// get value from a config file which is not best security practice
exists(env.getReturn().getMember("read_env"))
) and
source = env.getReturn().getReturn().asSource()
)
or
source.asExpr().isConstant()
or
exists(API::Node cn |
@@ -47,13 +60,13 @@ module DjangoConstantSecretKeyConfig {
source.asExpr() = cn.asExpr()
)
or
source.asExpr() =
API::moduleImport("os").getMember("environ").getASubscript().asSource().asExpr()
source = API::moduleImport("os").getMember("environ").getASubscript().asSource()
) and
// 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!
exists(source.getScope().getLocation().getFile().getRelativePath()) and
// special case for get_random_secret_key
not source.getScope().getLocation().getFile().getBaseName() = "crypto.py"
not source.getScope().getLocation().getFile().inStdlib() and
// special sanitize case for get_random_secret_key and django-environ
not source.getScope().getLocation().getFile().getBaseName() = ["environ.py", "crypto.py"]
}
/**
@@ -90,6 +103,7 @@ module DjangoConstantSecretKeyConfig {
sink = attr.getValue()
)
) and
exists(sink.getScope().getLocation().getFile().getRelativePath())
exists(sink.getScope().getLocation().getFile().getRelativePath()) and
not sink.getScope().getLocation().getFile().inStdlib()
}
}

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

@@ -27,6 +27,19 @@ module FlaskConstantSecretKeyConfig {
*/
predicate isSource(DataFlow::Node source) {
(
// because Env return an Exeption if there isan't any value (instead of none) we should check whether
// there is a default value of there is a config file which mostly these config files have a default value
exists(API::Node env | env = API::moduleImport("environ").getMember("Env") |
(
// has default value
exists(env.getKeywordParameter("SECRET_KEY").asSource())
or
// get value from a config file which is not best security practice
exists(env.getReturn().getMember("read_env"))
) and
source = env.getReturn().getReturn().asSource()
)
or
source.asExpr().isConstant()
or
exists(API::Node cn |
@@ -65,7 +78,10 @@ module FlaskConstantSecretKeyConfig {
source.asExpr() =
API::moduleImport("os").getMember("environ").getASubscript().asSource().asExpr()
) and
exists(source.getScope().getLocation().getFile().getRelativePath())
exists(source.getScope().getLocation().getFile().getRelativePath()) and
not source.getScope().getLocation().getFile().inStdlib() and
// special sanitize case for django-environ
not source.getScope().getLocation().getFile().getBaseName() = "crypto.py"
}
/**
@@ -102,14 +118,14 @@ module FlaskConstantSecretKeyConfig {
attr.getAttributeName() = ["secret_key", "jwt_secret_key"] and
sink = attr.getValue()
)
or
exists(SecretKeyAssignStmt e |
sink.asExpr() = e.getValue()
// | sameAsHardCodedConstantSanitizer(e.getTarget(0))
)
) and
exists(sink.getScope().getLocation().getFile().getRelativePath())
or
exists(SecretKeyAssignStmt e |
sink.asExpr() = e.getValue()
// | sameAsHardCodedConstantSanitizer(e.getTarget(0))
) and
exists(sink.getScope().getLocation().getFile().getRelativePath())
exists(sink.getScope().getLocation().getFile().getRelativePath()) and
not sink.getScope().getLocation().getFile().inStdlib()
}
// for case check whether SECRECT_KEY is empty or not or whether it is == to a hardcoded constant value
@@ -129,23 +145,32 @@ module FlaskConstantSecretKeyConfig {
*/
class SecretKeyAssignStmt extends AssignStmt {
SecretKeyAssignStmt() {
exists(string configFileName, string fileNamehelper, DataFlow::Node n1 |
fileNamehelper = flaskConfiFileName(n1) and
exists(string configFileName, string fileNamehelper, DataFlow::Node n1, File file |
fileNamehelper = [flaskConfiFileName(n1), flaskConfiFileName2(n1)] 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"
file = this.getLocation().getFile()
|
(
if configFileName = "__name__"
if fileNamehelper = "__name__"
then
this.getLocation().getFile().getShortName() =
flaskInstance().asSource().getLocation().getFile().getShortName()
else this.getLocation().getFile().getShortName().matches("%" + configFileName + "%")
file.getShortName() = flaskInstance().asSource().getLocation().getFile().getShortName()
else (
fileNamehelper.matches("%.py") and
file.getShortName().matches("%" + configFileName + "%") and
// after spliting, don't look at %py% pattern
configFileName != ".py"
or
// in case of referencing to a directory which then we must look for __init__.py file
not fileNamehelper.matches("%.py") and
file.getRelativePath()
.matches("%" + fileNamehelper.replaceAll(".", "/") + "/__init__.py")
)
) and
this.getTarget(0).toString() = ["SECRET_KEY", "JWT_SECRET_KEY"]
this.getTarget(0).(Name).getId() = ["SECRET_KEY", "JWT_SECRET_KEY"]
) and
exists(this.getScope().getLocation().getFile().getRelativePath())
exists(this.getScope().getLocation().getFile().getRelativePath()) and
not this.getScope().getLocation().getFile().inStdlib()
}
}
@@ -158,14 +183,27 @@ module FlaskConstantSecretKeyConfig {
* `app.config.from_object("configFileName.ClassName")`
*/
string flaskConfiFileName(API::CallNode cn) {
exists(API::Node app |
app = flaskInstance().getACall().getReturn() and
cn = app.getMember("config").getMember(["from_object", "from_pyfile"]).getACall() and
result =
[
cn.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText(),
cn.getParameter(0).asSink().asExpr().(Name).getId()
]
)
cn =
flaskInstance()
.getReturn()
.getMember("config")
.getMember(["from_object", "from_pyfile"])
.getACall() and
result =
[
cn.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText(),
cn.getParameter(0).asSink().asExpr().(Name).getId()
]
}
string flaskConfiFileName2(API::CallNode cn) {
cn =
API::moduleImport("flask")
.getMember("Flask")
.getASubclass*()
.getASuccessor*()
.getMember("from_object")
.getACall() and
result = cn.getParameter(0).asSink().asExpr().(StrConst).getText()
}
}

23
python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/examples/config1.py Normal file
View File

@@ -0,0 +1,23 @@
"""Flask App configuration."""
from os import environ
import os
import random
import configparser
FLASK_DEBUG = True
aConstant = 'CHANGEME2'
config = configparser.ConfigParser()
class Config:
SECRET_KEY = config["a"]["Secret"]
SECRET_KEY = config.get("key", "value")
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']

6
python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/examples/example_Django_unsafe.py
View File

@@ -5,9 +5,13 @@ from django.conf import global_settings
from django.urls import path
from django.http import HttpResponse
env = environ.Env(
SECRET_KEY=(bool, False)
SECRET_KEY=(str, "AConstantKey")
)
env.read_env(env_file='.env')
# following is not safe if there is a call to read_env or if there is default value in Env(..)
settings.SECRET_KEY = env('SECRET_KEY')
settings.configure(
DEBUG=True,

1
python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/examples/example_Flask_unsafe.py
View File

@@ -10,6 +10,7 @@ app.config.from_pyfile("config.py")
app.config.from_pyfile("config2.py")
app.config.from_object('config.Config')
app.config.from_object('config2.Config')
app.config.from_object('settings')
@app.route('/')

3
python/ql/src/experimental/Security/CWE-287-ConstantSecretKey/examples/settings/__init__.py Normal file
View File

@@ -0,0 +1,3 @@
import os
SECRET_KEY = "REDASH_COOKIE_SECRET"

2
python/ql/src/qlpack.yml
View File

@@ -1,5 +1,5 @@
name: codeql/python-queries
version: 0.7.3-dev
version: 0.7.4-dev
groups:
- python
- queries