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

Python: ORM: Add flow to collection/dict queries

Browse Source
This commit is contained in:
Rasmus Wriedt Larsen
2022-02-17 13:36:57 +01:00
parent 9cff4cbd1c
commit 9b458b54aa
4 changed files with 113 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
python/ql/lib/semmle/python/frameworks/Django.qll
View File

@@ -614,7 +614,7 @@ module PrivateDjango {
override predicate isDbFetch() { none() }
}
/** A method on a query-set or manager that returns an instance of a django model. */
/** A method call on a query-set or manager that returns an instance of a django model. */
private class QuerySetMethod extends InstanceSource, DataFlow::CallCfgNode {
API::Node modelClass;
string methodName;
@@ -630,6 +630,76 @@ module PrivateDjango {
override predicate isDbFetch() { not methodName = "create" }
}
/**
* A method call on a query-set or manager that returns a collection
* containing instances of a django models.
*/
class QuerySetMethodInstanceCollection extends DataFlow::CallCfgNode {
API::Node modelClass;
string methodName;
QuerySetMethodInstanceCollection() {
modelClass = subclassRef() and
this = querySetReturningMethod(modelClass, methodName).getACall() and
not methodName in ["none", "datetimes", "dates", "values", "values_list"]
or
// TODO: When we have flow-summaries, we should be able to model `values` and `values_list`
// Potentially by doing `synthetic ===store of list element==> <Model>.objects`, and then
// `.all()` just keeps that content, and `.first()` will do a read step (of the list element).
//
// So for `Model.objects.filter().exclude().first()` we would have
// 1: <Synthetic node for Model> ===store of list element==> Model.objects
// 2: Model.objects ==> Model.objects.filter()
// 3: Model.objects.filter() ==> Model.objects.filter().exclude()
// 4: Model.objects.filter().exclude() ===read of list element==> Model.objects.filter().exclude().first()
//
// This also means that `.none()` could clear contents. Right now we
// think that the result of `Model.objects.none().all()` can contain
// Model objects, but it will be empty due to the `.none()` part. Not
// that this is important, since no-one would need to write
// `.none().all()` code anyway, but it would be cool to be able to model it properly :D
//
//
// The big benefit is for how we could then model `values`/`values_list`. For example,
// `Model.objects.value_list(name, description)` would result in (for the attribute description)
// 0: <Synthetic node for Model> -- [attr description]
// 1: ==> Model.objects [ListElement, attr description]
// 2: ==> .value_list(...) [ListElement, TupleIndex 1]
//
// but for now, we just model a store step directly from the synthetic
// node to the method call.
// extra method on query-set/manager that does _not_ return a query-set
modelClass = subclassRef() and
methodName in ["iterator", "bulk_create"] and
this = [manager(modelClass), querySet(modelClass)].getMember(methodName).getACall()
}
/** Gets the model class that this is an instance source of. */
API::Node getModelClass() { result = modelClass }
/** Holds if this instance-source is fetching data from the DB. */
predicate isDbFetch() { not methodName = "bulk_create" }
}
/**
* A method call on a query-set or manager that returns a dictionary
* containing instances of a django models as the values.
*/
class QuerySetMethodInstanceDictValue extends DataFlow::CallCfgNode {
API::Node modelClass;
QuerySetMethodInstanceDictValue() {
modelClass = subclassRef() and
this = [manager(modelClass), querySet(modelClass)].getMember("in_bulk").getACall()
}
/** Gets the model class that this is an instance source of. */
API::Node getModelClass() { result = modelClass }
/** Holds if this instance-source is fetching data from the DB. */
predicate isDbFetch() { any() }
}
/**
* Gets a reference to an instance of `django.db.models.Model` class or any subclass,
* where `modelClass` specifies the class.
@@ -765,6 +835,22 @@ module PrivateDjango {
c.(DataFlow::AttributeContent).getAttribute() = fieldName and
nodeTo = call
)
or
// synthetic -> method-call that returns collection of ORM models (all/filter/...)
exists(API::Node modelClass |
nodeFrom.(SyntheticDjangoOrmModelNode).getModelClass() = modelClass and
nodeTo.(Model::QuerySetMethodInstanceCollection).getModelClass() = modelClass and
nodeTo.(Model::QuerySetMethodInstanceCollection).isDbFetch() and
c instanceof DataFlow::ListElementContent
)
or
// synthetic -> method-call that returns dictionary with ORM models as values
exists(API::Node modelClass |
nodeFrom.(SyntheticDjangoOrmModelNode).getModelClass() = modelClass and
nodeTo.(Model::QuerySetMethodInstanceDictValue).getModelClass() = modelClass and
nodeTo.(Model::QuerySetMethodInstanceDictValue).isDbFetch() and
c instanceof DataFlow::DictionaryElementAnyContent
)
}
override predicate jumpStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {

20
python/ql/test/library-tests/frameworks/django-orm/ReflectedXss.expected
View File

@@ -1,5 +1,7 @@
edges
| testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute age] | testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute age] |
| testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute age] | testapp/orm_security_tests.py:51:14:51:53 | ControlFlowNode for Attribute() [Attribute age] |
| testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute name] | testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute name] |
| testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute name] | testapp/orm_security_tests.py:47:14:47:53 | ControlFlowNode for Attribute() [Attribute name] |
| testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | testapp/orm_security_tests.py:22:23:22:34 | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | testapp/orm_security_tests.py:23:22:23:33 | ControlFlowNode for Attribute |
@@ -12,6 +14,14 @@ edges
| testapp/orm_security_tests.py:23:22:23:40 | ControlFlowNode for Subscript | testapp/orm_security_tests.py:23:9:23:14 | [post store] ControlFlowNode for person [Attribute age] |
| testapp/orm_security_tests.py:28:9:28:14 | ControlFlowNode for person [Attribute age] | testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute age] |
| testapp/orm_security_tests.py:28:9:28:14 | ControlFlowNode for person [Attribute name] | testapp/orm_security_tests.py:15:1:15:27 | [orm-model] Class Person [Attribute name] |
| testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute age] | testapp/orm_security_tests.py:43:62:43:67 | ControlFlowNode for person [Attribute age] |
| testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute name] | testapp/orm_security_tests.py:43:49:43:54 | ControlFlowNode for person [Attribute name] |
| testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute age] | testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute age] |
| testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute name] | testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute name] |
| testapp/orm_security_tests.py:43:49:43:54 | ControlFlowNode for person [Attribute name] | testapp/orm_security_tests.py:43:49:43:59 | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:43:49:43:59 | ControlFlowNode for Attribute | testapp/orm_security_tests.py:44:29:44:37 | ControlFlowNode for resp_text |
| testapp/orm_security_tests.py:43:62:43:67 | ControlFlowNode for person [Attribute age] | testapp/orm_security_tests.py:43:62:43:71 | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:43:62:43:71 | ControlFlowNode for Attribute | testapp/orm_security_tests.py:44:29:44:37 | ControlFlowNode for resp_text |
| testapp/orm_security_tests.py:47:14:47:53 | ControlFlowNode for Attribute() [Attribute name] | testapp/orm_security_tests.py:48:46:48:51 | ControlFlowNode for person [Attribute name] |
| testapp/orm_security_tests.py:48:46:48:51 | ControlFlowNode for person [Attribute name] | testapp/orm_security_tests.py:48:46:48:56 | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:48:46:48:56 | ControlFlowNode for Attribute | testapp/orm_security_tests.py:48:25:48:57 | ControlFlowNode for Attribute() |
@@ -47,6 +57,15 @@ nodes
| testapp/orm_security_tests.py:23:22:23:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
| testapp/orm_security_tests.py:28:9:28:14 | ControlFlowNode for person [Attribute age] | semmle.label | ControlFlowNode for person [Attribute age] |
| testapp/orm_security_tests.py:28:9:28:14 | ControlFlowNode for person [Attribute name] | semmle.label | ControlFlowNode for person [Attribute name] |
| testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute age] | semmle.label | SSA variable person [Attribute age] |
| testapp/orm_security_tests.py:42:13:42:18 | SSA variable person [Attribute name] | semmle.label | SSA variable person [Attribute name] |
| testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute age] | semmle.label | ControlFlowNode for Attribute() [List element, Attribute age] |
| testapp/orm_security_tests.py:42:23:42:42 | ControlFlowNode for Attribute() [List element, Attribute name] | semmle.label | ControlFlowNode for Attribute() [List element, Attribute name] |
| testapp/orm_security_tests.py:43:49:43:54 | ControlFlowNode for person [Attribute name] | semmle.label | ControlFlowNode for person [Attribute name] |
| testapp/orm_security_tests.py:43:49:43:59 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:43:62:43:67 | ControlFlowNode for person [Attribute age] | semmle.label | ControlFlowNode for person [Attribute age] |
| testapp/orm_security_tests.py:43:62:43:71 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| testapp/orm_security_tests.py:44:29:44:37 | ControlFlowNode for resp_text | semmle.label | ControlFlowNode for resp_text |
| testapp/orm_security_tests.py:47:14:47:53 | ControlFlowNode for Attribute() [Attribute name] | semmle.label | ControlFlowNode for Attribute() [Attribute name] |
| testapp/orm_security_tests.py:48:25:48:57 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| testapp/orm_security_tests.py:48:46:48:51 | ControlFlowNode for person [Attribute name] | semmle.label | ControlFlowNode for person [Attribute name] |
@@ -75,6 +94,7 @@ nodes
| testapp/orm_security_tests.py:121:25:121:36 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
subpaths
#select
| testapp/orm_security_tests.py:44:29:44:37 | ControlFlowNode for resp_text | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | testapp/orm_security_tests.py:44:29:44:37 | ControlFlowNode for resp_text | Cross-site scripting vulnerability due to $@. | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | a user-provided value |
| testapp/orm_security_tests.py:48:25:48:57 | ControlFlowNode for Attribute() | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | testapp/orm_security_tests.py:48:25:48:57 | ControlFlowNode for Attribute() | Cross-site scripting vulnerability due to $@. | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | a user-provided value |
| testapp/orm_security_tests.py:55:25:55:55 | ControlFlowNode for Attribute() | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | testapp/orm_security_tests.py:55:25:55:55 | ControlFlowNode for Attribute() | Cross-site scripting vulnerability due to $@. | testapp/orm_security_tests.py:19:12:19:18 | ControlFlowNode for request | a user-provided value |
| testapp/orm_security_tests.py:102:25:102:36 | ControlFlowNode for Attribute | testapp/orm_security_tests.py:95:37:95:43 | ControlFlowNode for request | testapp/orm_security_tests.py:102:25:102:36 | ControlFlowNode for Attribute | Cross-site scripting vulnerability due to $@. | testapp/orm_security_tests.py:95:37:95:43 | ControlFlowNode for request | a user-provided value |

2
python/ql/test/library-tests/frameworks/django-orm/testapp/orm_security_tests.py
View File

@@ -41,7 +41,7 @@ def person(request):
resp_text = "<h1>Persons:</h1>"
for person in Person.objects.all():
resp_text += "\n{} (age {})".format(person.name, person.age)
return HttpResponse(resp_text) # NOT OK
return HttpResponse(resp_text) # NOT OK
def show_name(request):
person = Person.objects.get(id=request.GET["id"])

10
python/ql/test/library-tests/frameworks/django-orm/testapp/orm_tests.py
View File

@@ -307,8 +307,8 @@ def test_load_single():
def test_load_many():
objs = TestLoad.objects.all()
for obj in objs:
SINK(obj.text) # $ MISSING: flow
SINK(objs[0].text) # $ MISSING: flow
SINK(obj.text) # $ flow="SOURCE, l:-10 -> obj.text"
SINK(objs[0].text) # $ flow="SOURCE, l:-11 -> objs[0].text"
def test_load_many_skip():
objs = TestLoad.objects.all()[5:]
@@ -323,8 +323,8 @@ def test_load_qs_chain_single():
def test_load_qs_chain_many():
objs = TestLoad.objects.all().filter(text__contains="s").exclude(text=None)
for obj in objs:
SINK(obj.text) # $ MISSING: flow
SINK(objs[0].text) # $ MISSING: flow
SINK(obj.text) # $ flow="SOURCE, l:-26 -> obj.text"
SINK(objs[0].text) # $ flow="SOURCE, l:-27 -> objs[0].text"
def test_load_values():
# see https://docs.djangoproject.com/en/4.0/ref/models/querysets/#django.db.models.query.QuerySet.values
@@ -363,7 +363,7 @@ def test_load_in_bulk():
d = TestLoad.objects.in_bulk([1])
for val in d.values():
SINK(val.text) # $ MISSING: flow
SINK(d[1].text) # $ MISSING: flow
SINK(d[1].text) # $ flow="SOURCE, l:-66 -> d[1].text"
# Good resources: