Skip to content

C#: Re-factor the experimental PotentialTimeBomb to use new API. #12909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michaelnebel merged 2 commits into from
May 9, 2023
Merged

C#: Re-factor the experimental PotentialTimeBomb to use new API. #12909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2529b8
C#: Re-factor the PotentialTimeBomb query to use the new API.
michaelnebel Apr 24, 2023
d01674f
C#: Update expected test output.
michaelnebel Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 58 additions & 58 deletions csharp/ql/src/experimental/Security Features/backdoor/PotentialTimeBomb.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,16 @@
*/

import csharp
import DataFlow
import Flow::PathGraph

query predicate nodes = PathGraph::nodes/3;

query predicate edges(DataFlow::PathNode a, DataFlow::PathNode b) {
PathGraph::edges(a, b)
query predicate edges(Flow::PathNode a, Flow::PathNode b) {
Flow::PathGraph::edges(a, b)
or
exists(
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable conf1,
FlowsFromTimeSpanArithmeticToTimeComparisonCallable conf2
|
conf1 = a.getConfiguration() and
conf1.isSink(a.getNode()) and
conf2 = b.getConfiguration() and
b.isSource()
)
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig::isSink(a.getNode()) and
FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig::isSource(b.getNode())
or
exists(
FlowsFromTimeSpanArithmeticToTimeComparisonCallable conf1,
FlowsFromTimeComparisonCallableToSelectionStatementCondition conf2
|
conf1 = a.getConfiguration() and
conf1.isSink(a.getNode()) and
conf2 = b.getConfiguration() and
b.isSource()
)
FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig::isSink(a.getNode()) and
FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig::isSource(b.getNode())
}

/**
Expand Down Expand Up @@ -78,22 +62,19 @@ class DateTimeStruct extends Struct {
}

/**
* Dataflow configuration to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
* Configuration to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
*/
private class FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable extends TaintTracking::Configuration
private module FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig implements
DataFlow::ConfigSig
{
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable() {
this = "FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable"
}

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
exists(Call call, GetLastWriteTimeMethod m |
m.getACall() = call and
source.asExpr() = call
)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(Call call, DateTimeStruct dateTime |
call.getAChild*() = sink.asExpr() and
call = dateTime.getATimeSpanArithmeticCallable().getACall()
Expand All @@ -102,21 +83,24 @@ private class FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable extend
}

/**
* Dataflow configuration to find flow from a DateTime arithmetic operation to a DateTime comparison operation
* Tainttracking module to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
*/
private class FlowsFromTimeSpanArithmeticToTimeComparisonCallable extends TaintTracking::Configuration
{
FlowsFromTimeSpanArithmeticToTimeComparisonCallable() {
this = "FlowsFromTimeSpanArithmeticToTimeComparisonCallable"
}
private module FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable =
TaintTracking::Global<FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig>;

override predicate isSource(DataFlow::Node source) {
/**
* Configuration to find flow from a DateTime arithmetic operation to a DateTime comparison operation
*/
private module FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig implements
DataFlow::ConfigSig
{
predicate isSource(DataFlow::Node source) {
exists(DateTimeStruct dateTime, Call call | source.asExpr() = call |
call = dateTime.getATimeSpanArithmeticCallable().getACall()
)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(Call call, DateTimeStruct dateTime |
call.getAnArgument().getAChild*() = sink.asExpr() and
call = dateTime.getAComparisonCallable().getACall()
Expand All @@ -125,53 +109,69 @@ private class FlowsFromTimeSpanArithmeticToTimeComparisonCallable extends TaintT
}

/**
* Dataflow configuration to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
* Tainttracking module to find flow from a DateTime arithmetic operation to a DateTime comparison operation
*/
private class FlowsFromTimeComparisonCallableToSelectionStatementCondition extends TaintTracking::Configuration
{
FlowsFromTimeComparisonCallableToSelectionStatementCondition() {
this = "FlowsFromTimeComparisonCallableToSelectionStatementCondition"
}
private module FlowsFromTimeSpanArithmeticToTimeComparisonCallable =
TaintTracking::Global<FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig>;

override predicate isSource(DataFlow::Node source) {
/**
* Configuration to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
*/
private module FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig implements
DataFlow::ConfigSig
{
predicate isSource(DataFlow::Node source) {
exists(DateTimeStruct dateTime, Call call | source.asExpr() = call |
call = dateTime.getAComparisonCallable().getACall()
)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(SelectionStmt sel | sel.getCondition().getAChild*() = sink.asExpr())
}
}

/**
* Tainttracking module to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
*/
private module FlowsFromTimeComparisonCallableToSelectionStatementCondition =
TaintTracking::Global<FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig>;

private module Flow =
DataFlow::MergePathGraph3<FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::PathNode,
FlowsFromTimeSpanArithmeticToTimeComparisonCallable::PathNode,
FlowsFromTimeComparisonCallableToSelectionStatementCondition::PathNode,
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::PathGraph,
FlowsFromTimeSpanArithmeticToTimeComparisonCallable::PathGraph,
FlowsFromTimeComparisonCallableToSelectionStatementCondition::PathGraph>;

/**
* Holds if the last file modification date from the call to getLastWriteTimeMethodCall will be used in a DateTime arithmetic operation timeArithmeticCall,
* which is then used for a DateTime comparison timeComparisonCall and the result flows to a Selection statement which is likely a TimeBomb trigger
*/
predicate isPotentialTimeBomb(
DataFlow::PathNode pathSource, DataFlow::PathNode pathSink, Call getLastWriteTimeMethodCall,
Flow::PathNode pathSource, Flow::PathNode pathSink, Call getLastWriteTimeMethodCall,
Call timeArithmeticCall, Call timeComparisonCall, SelectionStmt selStatement
) {
exists(
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable config1, Node sink,
DateTimeStruct dateTime, FlowsFromTimeSpanArithmeticToTimeComparisonCallable config2,
Node sink2, FlowsFromTimeComparisonCallableToSelectionStatementCondition config3, Node sink3
|
pathSource.getNode() = exprNode(getLastWriteTimeMethodCall) and
config1.hasFlow(exprNode(getLastWriteTimeMethodCall), sink) and
exists(DataFlow::Node sink, DateTimeStruct dateTime, DataFlow::Node sink2, DataFlow::Node sink3 |
pathSource.getNode() = DataFlow::exprNode(getLastWriteTimeMethodCall) and
FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::flow(DataFlow::exprNode(getLastWriteTimeMethodCall),
sink) and
timeArithmeticCall = dateTime.getATimeSpanArithmeticCallable().getACall() and
timeArithmeticCall.getAChild*() = sink.asExpr() and
config2.hasFlow(exprNode(timeArithmeticCall), sink2) and
FlowsFromTimeSpanArithmeticToTimeComparisonCallable::flow(DataFlow::exprNode(timeArithmeticCall),
sink2) and
timeComparisonCall = dateTime.getAComparisonCallable().getACall() and
timeComparisonCall.getAnArgument().getAChild*() = sink2.asExpr() and
config3.hasFlow(exprNode(timeComparisonCall), sink3) and
FlowsFromTimeComparisonCallableToSelectionStatementCondition::flow(DataFlow::exprNode(timeComparisonCall),
sink3) and
selStatement.getCondition().getAChild*() = sink3.asExpr() and
pathSink.getNode() = sink3
)
}

from
DataFlow::PathNode source, DataFlow::PathNode sink, Call getLastWriteTimeMethodCall,
Flow::PathNode source, Flow::PathNode sink, Call getLastWriteTimeMethodCall,
Call timeArithmeticCall, Call timeComparisonCall, SelectionStmt selStatement
where
isPotentialTimeBomb(source, sink, getLastWriteTimeMethodCall, timeArithmeticCall,
Expand Down
18 changes: 11 additions & 7 deletions csharp/ql/test/experimental/Security Features/backdoor/PotentialTimeBomb.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
nodes
| test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | semmle.label | call to method GetLastWriteTime : DateTime |
| test.cs:71:13:71:71 | call to method CompareTo | semmle.label | call to method CompareTo |
| test.cs:71:13:71:71 | call to method CompareTo : Int32 | semmle.label | call to method CompareTo : Int32 |
| test.cs:71:13:71:76 | ... >= ... | semmle.label | ... >= ... |
| test.cs:71:36:71:48 | access to local variable lastWriteTime | semmle.label | access to local variable lastWriteTime |
| test.cs:71:36:71:70 | call to method AddHours | semmle.label | call to method AddHours |
subpaths
edges
| test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:36:71:48 | access to local variable lastWriteTime |
| test.cs:71:13:71:71 | call to method CompareTo : Int32 | test.cs:71:13:71:76 | ... >= ... |
| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:13:71:71 | call to method CompareTo |
| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 |
| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:36:71:70 | call to method AddHours |
| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo |
| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo : Int32 |
| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:36:71:70 | call to method AddHours |
#select
| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:76 | ... >= ... | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
nodes
| test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | semmle.label | call to method GetLastWriteTime : DateTime |
| test.cs:71:13:71:71 | call to method CompareTo | semmle.label | call to method CompareTo |
| test.cs:71:13:71:71 | call to method CompareTo : Int32 | semmle.label | call to method CompareTo : Int32 |
| test.cs:71:13:71:76 | ... >= ... | semmle.label | ... >= ... |
| test.cs:71:36:71:48 | access to local variable lastWriteTime | semmle.label | access to local variable lastWriteTime |
| test.cs:71:36:71:70 | call to method AddHours | semmle.label | call to method AddHours |