This commit is contained in:
amammad
2023-06-29 20:55:51 +10:00
parent e3e0307db7
commit 7a17b99c17
15 changed files with 484 additions and 251 deletions

View File

@@ -15,11 +15,11 @@
</p>
</recommendation>
<example>
<sample src="example_bad.py" />
<sample src="example_good.py" />
<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" />
</example>
<references>
<li>

View File

@@ -1,219 +0,0 @@
/**
* @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,58 @@
/**
* @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/flask-constant-secret-key
* @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
import WebAppConstantSecretKeyDjango
import WebAppConstantSecretKeyFlask
newtype TFrameWork =
Flask() or
Django()
module WebAppConstantSecretKeyConfig implements DataFlow::StateConfigSig {
class FlowState = TFrameWork;
predicate isSource(DataFlow::Node source, FlowState state) {
state = Flask() and FlaskConstantSecretKeyConfig::isSource(source)
or
state = Django() and DjangoConstantSecretKeyConfig::isSource(source)
}
predicate isSink(DataFlow::Node sink, FlowState state) {
state = Flask() and FlaskConstantSecretKeyConfig::isSink(sink)
or
state = Django() and DjangoConstantSecretKeyConfig::isSink(sink)
}
predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
predicate isAdditionalFlowStep(
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
) {
none()
}
}
module WebAppConstantSecretKey = TaintTracking::GlobalWithState<WebAppConstantSecretKeyConfig>;
import WebAppConstantSecretKey::PathGraph
from WebAppConstantSecretKey::PathNode source, WebAppConstantSecretKey::PathNode sink
where WebAppConstantSecretKey::flowPath(source, sink)
select sink, source, sink, "The SECRET_KEY config variable is assigned by $@.", source,
" this constant String"

View File

@@ -0,0 +1,95 @@
import python
import experimental.semmle.python.Concepts
import semmle.python.dataflow.new.DataFlow
import semmle.python.ApiGraphs
import semmle.python.dataflow.new.TaintTracking
module DjangoConstantSecretKeyConfig {
/**
* 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
source.asExpr() =
API::moduleImport("os").getMember("environ").getASubscript().asSource().asExpr()
) 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"
}
/**
* A sink like following SECRET_KEY Assignments
* ```python
*from django.conf import settings
*settings.configure(
* SECRET_KEY="constant",
*)
* # or
*settings.SECRET_KEY = "constant"
* ```
*/
predicate isSink(DataFlow::Node sink) {
exists(API::moduleImport("django")) and
(
exists(AssignStmt e | e.getTarget(0).(Name).getId() = ["SECRET_KEY", "SECRET_KEY_FALLBACKS"] |
sink.asExpr() = e.getValue()
)
or
exists(API::Node settings |
settings =
API::moduleImport("django").getMember("conf").getMember(["global_settings", "settings"]) and
sink =
settings
.getMember("configure")
.getKeywordParameter(["SECRET_KEY_FALLBACKS", "SECRET_KEY"])
.asSink()
)
or
exists(API::Node n, DataFlow::AttrWrite attr |
attr.getObject().getALocalSource() = n.asSource() and
attr.getAttributeName() = ["SECRET_KEY_FALLBACKS", "SECRET_KEY"] and
sink = attr.getValue()
)
) and
exists(sink.getScope().getLocation().getFile().getRelativePath())
}
}

View File

@@ -0,0 +1,171 @@
import python
import experimental.semmle.python.Concepts
import semmle.python.dataflow.new.DataFlow
import semmle.python.ApiGraphs
import semmle.python.dataflow.new.TaintTracking
/**
* 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 {
/**
* `flask.Flask()`
*/
API::Node flaskInstance() {
result = API::moduleImport("flask").getMember("Flask").getASubclass*()
}
/**
* 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
source.asExpr() =
API::moduleImport("os").getMember("environ").getASubscript().asSource().asExpr()
) and
exists(source.getScope().getLocation().getFile().getRelativePath())
}
/**
* 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().getReturn() |
sink =
[
n.getMember("config").getSubscript(["SECRET_KEY", "JWT_SECRET_KEY"]).asSink(),
n.getMember("config")
.getMember(["update", "from_mapping"])
.getACall()
.getArgByName(["SECRET_KEY", "JWT_SECRET_KEY"])
]
)
or
exists(API::Node n, DataFlow::AttrWrite attr | n = flaskInstance() |
attr.getObject().getALocalSource() = n.getACall() and
attr.getAttributeName() = ["secret_key", "jwt_secret_key"] and
sink = attr.getValue()
)
) 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())
}
// for case check whether SECRECT_KEY is empty or not or whether it is == to a hardcoded constant value
// i don't know why I can't reach from an expression to an If subExpression node
predicate sameAsHardCodedConstantSanitizer(
DataFlow::Node source, DataFlow::Node sink, SecretKeyAssignStmt e, If i
) {
source.asExpr() = e.getTarget(0).getAChildNode() and
// source.getLocation().toString().matches("%config3.py%")and
DataFlow::localFlow(source, sink) and
sink.asExpr() = i.getASubExpression().getAChildNode*().(Compare)
}
/**
* Assignments like `SECRET_KEY = ConstantValue`
* and `SECRET_KEY` file must be the Location that is specified in argument of `from_object` or `from_pyfile` methods
*/
class SecretKeyAssignStmt extends AssignStmt {
SecretKeyAssignStmt() {
exists(string configFileName, string fileNamehelper, DataFlow::Node n1 |
fileNamehelper = flaskConfiFileName(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"
|
(
if configFileName = "__name__"
then
this.getLocation().getFile().getShortName() =
flaskInstance().asSource().getLocation().getFile().getShortName()
else this.getLocation().getFile().getShortName().matches("%" + configFileName + "%")
) and
this.getTarget(0).toString() = ["SECRET_KEY", "JWT_SECRET_KEY"]
) and
exists(this.getScope().getLocation().getFile().getRelativePath())
}
}
/**
* A helper predicate that specify where the Flask `SECRET_KEY` variable location is defined.
* In Flask we have config files that specify the location of `SECRET_KEY` variable initialization
* and the name of these files are determined by
* `app.config.from_pyfile("configFileName.py")`
* or
* `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()
]
)
}
}

View File

@@ -1,18 +0,0 @@
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,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']

View File

@@ -0,0 +1,10 @@
"""Flask App configuration."""
import environ
aConstant = 'CHANGEME2'
SECRET_KEY = aConstant
class Config:
SECRET_KEY = aConstant

View File

@@ -0,0 +1,7 @@
"""Flask App configuration."""
# General Config
FLASK_DEBUG = True
SECRET_KEY = "CHANGEME5"
if SECRET_KEY == "CHANGEaME5":
raise "not possible"

View File

@@ -0,0 +1,32 @@
import sys
from django.conf import settings
from django.conf import global_settings
from django.urls import path
from django.http import HttpResponse
from django.core.management.utils import get_random_secret_key
settings.configure(
DEBUG=True,
SECRET_KEY=get_random_secret_key(),
ROOT_URLCONF=__name__,
)
global_settings.SECRET_KEY = get_random_secret_key()
settings.SECRET_KEY = get_random_secret_key()
def home(request):
return HttpResponse(settings.SECRET_KEY)
urlpatterns = [
path("", home),
]
if __name__ == "__main__":
from django.core.management import execute_from_command_line
if len(sys.argv) == 1:
sys.argv.append("runserver")
sys.argv.append("8080")
execute_from_command_line(sys.argv)

View File

@@ -0,0 +1,35 @@
import sys
import environ
from django.conf import settings
from django.conf import global_settings
from django.urls import path
from django.http import HttpResponse
env = environ.Env(
SECRET_KEY=(bool, False)
)
settings.configure(
DEBUG=True,
SECRET_KEY="constant1",
ROOT_URLCONF=__name__,
)
global_settings.SECRET_KEY = "constant2"
settings.SECRET_KEY = "constant3"
def home(request):
return HttpResponse(settings.SECRET_KEY)
urlpatterns = [
path("", home),
]
if __name__ == "__main__":
from django.core.management import execute_from_command_line
if len(sys.argv) == 1:
sys.argv.append("runserver")
sys.argv.append("8080")
execute_from_command_line(sys.argv)

View File

@@ -2,17 +2,16 @@ from flask import Flask, session
from secrets import token_hex
app = Flask(__name__)
app.config['SECRET_KEY'] = token_hex(64)
app.secret_key = token_hex(16)
app.config.from_pyfile("config3.py")
@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
session['logged_in'] = 'value'
# debugging whether secret_key is secure or not
return app.secret_key
if __name__ == '__main__':

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_pyfile("config2.py")
app.config.from_object('config.Config')
app.config.from_object('config2.Config')
@app.route('/')
def DEB_EX():
if 'logged_in' not in session:
session['logged_in'] = 'value'
# debugging whether secret_key is secure or not
return app.secret_key, session.get('logged_in')
if __name__ == '__main__':
app.run()

View File

@@ -0,0 +1,18 @@
from flask import Flask, session
app = Flask(__name__)
aConstant = 'CHANGEME1'
SECRET_KEY = aConstant
app.config.from_object(__name__)
@app.route('/')
def DEB_EX():
if 'logged_in' not in session:
session['logged_in'] = 'value'
# debugging whether secret_key is secure or not
return app.secret_key, session.__str__()
if __name__ == '__main__':
app.run()

View File

@@ -2,10 +2,8 @@ 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)
SECRET_KEY = token_hex(16)
@app.route('/')