Python: promote nosql query #14070
Python: Refactor to allow customizations
Also use new DataFlow API
- Loading branch information
commit 087961d179fa9809bf379aff916644605654fe6b
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import python | ||
| import semmle.python.dataflow.new.DataFlow | ||
| import semmle.python.dataflow.new.RemoteFlowSources | ||
| import semmle.python.Concepts | ||
|
|
||
| module NoSqlInjection { | ||
| private newtype TFlowState = | ||
| TStringInput() or | ||
| TDictInput() | ||
|
|
||
| abstract class FlowState extends TFlowState { | ||
| abstract string toString(); | ||
| } | ||
|
|
||
| class StringInput extends FlowState, TStringInput { | ||
| override string toString() { result = "StringInput" } | ||
| } | ||
|
|
||
| class DictInput extends FlowState, TDictInput { | ||
| override string toString() { result = "DictInput" } | ||
| } | ||
|
|
||
| abstract class StringSource extends DataFlow::Node { } | ||
|
|
||
| abstract class DictSource extends DataFlow::Node { } | ||
|
|
||
| abstract class StringSink extends DataFlow::Node { } | ||
|
|
||
| abstract class DictSink extends DataFlow::Node { } | ||
|
|
||
| abstract class StringToDictConversion extends DataFlow::Node { | ||
| abstract DataFlow::Node getAnInput(); | ||
|
|
||
| abstract DataFlow::Node getOutput(); | ||
| } | ||
|
|
||
| class RemoteFlowSourceAsStringSource extends RemoteFlowSource, StringSource { } | ||
|
|
||
| class NoSqlQueryAsDictSink extends DictSink { | ||
| NoSqlQueryAsDictSink() { this = any(NoSqlQuery noSqlQuery).getQuery() } | ||
| } | ||
|
|
||
| class JsonDecoding extends Decoding, StringToDictConversion { | ||
| JsonDecoding() { this.getFormat() = "JSON" } | ||
|
|
||
| override DataFlow::Node getAnInput() { result = Decoding.super.getAnInput() } | ||
|
|
||
| override DataFlow::Node getOutput() { result = Decoding.super.getOutput() } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,53 +1,48 @@ | ||
| import python | ||
| import semmle.python.dataflow.new.DataFlow | ||
| import semmle.python.dataflow.new.TaintTracking | ||
| import semmle.python.Concepts | ||
| private import NoSQLInjectionCustomizations::NoSqlInjection as C | ||
|
|
||
| module Config implements DataFlow::StateConfigSig { | ||
| class FlowState = C::FlowState; | ||
|
|
||
| predicate isSource(DataFlow::Node source, FlowState state) { | ||
| source instanceof C::StringSource and | ||
| state instanceof C::StringInput | ||
| or | ||
| source instanceof C::DictSource and | ||
| state instanceof C::DictInput | ||
| } | ||
|
|
||
| predicate isSink(DataFlow::Node source, FlowState state) { | ||
| source instanceof C::StringSink and | ||
| state instanceof C::StringInput | ||
| or | ||
| source instanceof C::DictSink and | ||
| state instanceof C::DictInput | ||
| } | ||
|
|
||
| predicate isBarrier(DataFlow::Node node, FlowState state) { | ||
| // Block `StringInput` paths here, since they change state to `DictInput` | ||
| exists(C::StringToDictConversion c | node = c.getOutput()) and | ||
| state instanceof C::StringInput | ||
| } | ||
|
|
||
| predicate isAdditionalFlowStep( | ||
| DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo | ||
| ) { | ||
| exists(C::StringToDictConversion c | | ||
| nodeFrom = c.getAnInput() and | ||
| nodeTo = c.getOutput() | ||
| ) and | ||
| stateFrom instanceof C::StringInput and | ||
| stateTo instanceof C::DictInput | ||
| } | ||
|
|
||
| predicate isBarrier(DataFlow::Node node) { | ||
| node = any(NoSqlSanitizer noSqlSanitizer).getAnInput() | ||
| } | ||
| } | ||
|
|
||
| module Flow = TaintTracking::GlobalWithState<Config>; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did this
as Cpattern come from? I have not seen it before, and would have thought we could do without it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is to communicate the
FlowStateclass from the imported moduleCto the declared moduleNoSqlInjectionConfig. Otherwise, we should rename theFlowStateclass insideC, but that seems a bit wrong..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. C for customizations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that was the thinking. I guess it is a little too short to convey anything..