diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index 890e45a2ab5..1cf7d2573f8 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/python/ql/lib/semmle/python/frameworks/Flask.qll @@ -408,21 +408,21 @@ module Flask { } } + private API::Node requestFileStorage() { + // TODO: This approach for identifying member-access is very adhoc, and we should + // be able to do something more structured for providing modeling of the members + // of a container-object. + result = request().getMember("files").getASubscript() + or + result = request().getMember("files").getMember("get").getReturn() + or + result = request().getMember("files").getMember("getlist").getReturn().getASubscript() + } + /** An `FileStorage` instance that originates from a flask request. */ private class FlaskRequestFileStorageInstances extends Werkzeug::FileStorage::InstanceSource { FlaskRequestFileStorageInstances() { - // TODO: This approach for identifying member-access is very adhoc, and we should - // be able to do something more structured for providing modeling of the members - // of a container-object. - exists(API::Node files | files = request().getMember("files") | - this.asCfgNode().(SubscriptNode).getObject() = - files.getAValueReachableFromSource().asCfgNode() - or - this = files.getMember("get").getACall() - or - this.asCfgNode().(SubscriptNode).getObject() = - files.getMember("getlist").getReturn().getAValueReachableFromSource().asCfgNode() - ) + this = requestFileStorage().getAValueReachableFromSource() } } diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index dc93677dea0..ed61a8d9683 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -1725,39 +1725,21 @@ private module StdlibPrivate { API::Node getlistResult() { result = getlistRef().getReturn() } /** Gets a reference to a list of fields. */ - private DataFlow::TypeTrackingNode fieldList(DataFlow::TypeTracker t) { - t.start() and - // TODO: Should have better handling of subscripting - result.asCfgNode().(SubscriptNode).getObject() = - instance().getAValueReachableFromSource().asCfgNode() + API::Node fieldList() { + result = getlistResult() or - exists(DataFlow::TypeTracker t2 | result = fieldList(t2).track(t2, t)) - } - - /** Gets a reference to a list of fields. */ - DataFlow::Node fieldList() { - result = getlistResult().getAValueReachableFromSource() or - result = getvalueResult().getAValueReachableFromSource() or - fieldList(DataFlow::TypeTracker::end()).flowsTo(result) + result = getvalueResult() + or + result = instance().getASubscript() } /** Gets a reference to a field. */ - private DataFlow::TypeTrackingNode field(DataFlow::TypeTracker t) { - t.start() and - // TODO: Should have better handling of subscripting - result.asCfgNode().(SubscriptNode).getObject() = - [instance().getAValueReachableFromSource(), fieldList()].asCfgNode() + API::Node field() { + result = getfirstResult() or - exists(DataFlow::TypeTracker t2 | result = field(t2).track(t2, t)) - } - - /** Gets a reference to a field. */ - DataFlow::Node field() { - result = getfirstResult().getAValueReachableFromSource() + result = getvalueResult() or - result = getvalueResult().getAValueReachableFromSource() - or - field(DataFlow::TypeTracker::end()).flowsTo(result) + result = [instance(), fieldList()].getASubscript() } private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep { @@ -1780,11 +1762,13 @@ private module StdlibPrivate { ) or // Indexing - nodeFrom in [instance().getAValueReachableFromSource(), fieldList()] and + nodeFrom in [ + instance().getAValueReachableFromSource(), fieldList().getAValueReachableFromSource() + ] and nodeTo.asCfgNode().(SubscriptNode).getObject() = nodeFrom.asCfgNode() or // Attributes on Field - nodeFrom = field() and + nodeFrom = field().getAValueReachableFromSource() and exists(DataFlow::AttrRead read | nodeTo = read and read.getObject() = nodeFrom | read.getAttributeName() in ["value", "file", "filename"] ) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Django.qll b/python/ql/src/experimental/semmle/python/frameworks/Django.qll index d9822014466..441911866f3 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Django.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Django.qll @@ -85,35 +85,20 @@ private module ExperimentalPrivateDjango { DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } /** Gets a reference to a header instance. */ - private DataFlow::LocalSourceNode headerInstance(DataFlow::TypeTracker t) { - t.start() and - ( - exists(SubscriptNode subscript | - subscript.getObject() = - baseClassRef().getReturn().getAValueReachableFromSource().asCfgNode() and - result.asCfgNode() = subscript - ) - or - result.(DataFlow::AttrRead).getObject() = - baseClassRef().getReturn().getAValueReachableFromSource() - ) + API::Node headerInstance() { + result = baseClassRef().getReturn().getASubscript() or - exists(DataFlow::TypeTracker t2 | result = headerInstance(t2).track(t2, t)) - } - - /** Gets a reference to a header instance use. */ - private DataFlow::Node headerInstance() { - headerInstance(DataFlow::TypeTracker::end()).flowsTo(result) + result = baseClassRef().getReturn().getAMember() } /** Gets a reference to a header instance call with `__setitem__`. */ - private DataFlow::Node headerSetItemCall() { + API::Node headerSetItem() { result = headerInstance() and - result.(DataFlow::AttrRead).getAttributeName() = "__setitem__" + result.asSource().(DataFlow::AttrRead).getAttributeName() = "__setitem__" } class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { - DjangoResponseSetItemCall() { this.getFunction() = headerSetItemCall() } + DjangoResponseSetItemCall() { this = headerSetItem().getACall() } override DataFlow::Node getNameArg() { result = this.getArg(0) } @@ -124,7 +109,8 @@ private module ExperimentalPrivateDjango { DataFlow::Node headerInput; DjangoResponseDefinition() { - this.asCfgNode().(DefinitionNode) = headerInstance().asCfgNode() and + this.asCfgNode().(DefinitionNode) = + headerInstance().getAValueReachableFromSource().asCfgNode() and headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue() }