Skip to content

C#: Re-use asPartialModel in utility and test code. #8449

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,45 @@ class JumpReturnKind extends ReturnKind, TJumpReturnKind {

class DataFlowCallable extends DotNet::Callable {
DataFlowCallable() { this.isUnboundDeclaration() }

private string parameterQualifiedTypeNamesToString() {
result =
concat(Parameter p, int i |
p = this.getParameter(i)
|
p.getType().getQualifiedName(), "," order by i
)
}

/** Holds if a summary should apply for all overrides of this. */
predicate isBaseCallableOrPrototype() {
this.getDeclaringType() instanceof Interface
or
exists(Modifiable m | m = [this.(Modifiable), this.(Accessor).getDeclaration()] |
m.isAbstract()
or
this.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
)
}

/** Gets a string representing whether a summary should apply for all overrides of this. */
private string getCallableOverride() {
if this.isBaseCallableOrPrototype() then result = "true" else result = "false"
}

/** Computes the first 6 columns for CSV rows. */
string asPartialModel() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This predicate and isBaseCallableOrPrototype will now be public (since DataFlowCallable is), which I don't think we want. Instead it would be better to add them as top-level predicates in DataFlowPrivate.qll.

exists(string namespace, string type |
this.getDeclaringType().hasQualifiedName(namespace, type) and
result =
namespace + ";" //
+ type + ";" //
+ this.getCallableOverride() + ";" //
+ this.getName() + ";" //
+ "(" + this.parameterQualifiedTypeNamesToString() + ")" //
+ /* ext + */ ";" //
)
}
}

/** A call relevant for data flow. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1022,46 +1022,6 @@ module Private {
}
}

/** Provides a query predicate for outputting a set of relevant flow summaries. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it makes sense to have here, in case other languages want to use it later.

module TestOutput {
/** A flow summary to include in the `summary/3` query predicate. */
abstract class RelevantSummarizedCallable extends SummarizedCallable {
/** Gets the string representation of this callable used by `summary/1`. */
abstract string getCallableCsv();

/** Holds if flow is propagated between `input` and `output`. */
predicate relevantSummary(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
this.propagatesFlow(input, output, preservesValue)
}
}

/** Render the kind in the format used in flow summaries. */
private string renderKind(boolean preservesValue) {
preservesValue = true and result = "value"
or
preservesValue = false and result = "taint"
}

/**
* A query predicate for outputting flow summaries in semi-colon separated format in QL tests.
* The syntax is: "namespace;type;overrides;name;signature;ext;inputspec;outputspec;kind",
* ext is hardcoded to empty.
*/
query predicate summary(string csv) {
exists(
RelevantSummarizedCallable c, SummaryComponentStack input, SummaryComponentStack output,
boolean preservesValue
|
c.relevantSummary(input, output, preservesValue) and
csv =
c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
getComponentStackCsv(output) + ";" + renderKind(preservesValue)
)
}
}

/**
* Provides query predicates for rendering the generated data flow graph for
* a summarized callable.
Expand Down
6 changes: 3 additions & 3 deletions csharp/ql/src/utils/model-generator/ModelGeneratorUtils.qll
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ predicate isRelevantContent(DataFlow::Content c) {
bindingset[input, output, kind]
string asSummaryModel(TargetApi api, string input, string output, string kind) {
result =
asPartialModel(api) + input + ";" //
api.asPartialModel() + input + ";" //
+ output + ";" //
+ kind
}
Expand All @@ -59,13 +59,13 @@ string asTaintModel(TargetApi api, string input, string output) {
*/
bindingset[input, kind]
string asSinkModel(TargetApi api, string input, string kind) {
result = asPartialModel(api) + input + ";" + kind
result = api.asPartialModel() + input + ";" + kind
}

/**
* Gets the source model for `api` with `output` and `kind`.
*/
bindingset[output, kind]
string asSourceModel(TargetApi api, string output, string kind) {
result = asPartialModel(api) + output + ";" + kind
result = api.asPartialModel() + output + ";" + kind
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,58 +7,19 @@ private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
private predicate isRelevantForModels(Callable api) { not api instanceof MainMethod }

/**
* A class of Callables that are relevant for generating summary, source and sinks models for.
* A class of DataFlowCallables that are relevant for generating summary, source and sinks models.
*
* In the Standard library and 3rd party libraries it the Callables that can be called
* from outside the library itself.
*/
class TargetApi extends Callable {
class TargetApi extends DataFlowCallable {
TargetApi() {
[this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic() and
this.fromSource() and
isRelevantForModels(this)
}
}

private string parameterQualifiedTypeNamesToString(TargetApi api) {
result =
concat(Parameter p, int i |
p = api.getParameter(i)
|
p.getType().getQualifiedName(), "," order by i
)
}

/** Holds if the summary should apply for all overrides of this. */
private predicate isBaseCallableOrPrototype(TargetApi api) {
api.getDeclaringType() instanceof Interface
or
exists(Modifiable m | m = [api.(Modifiable), api.(Accessor).getDeclaration()] |
m.isAbstract()
or
api.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
)
}

/** Gets a string representing whether the summary should apply for all overrides of this. */
private string getCallableOverride(TargetApi api) {
if isBaseCallableOrPrototype(api) then result = "true" else result = "false"
}

/** Computes the first 6 columns for CSV rows. */
string asPartialModel(TargetApi api) {
exists(string namespace, string type |
api.getDeclaringType().hasQualifiedName(namespace, type) and
result =
namespace + ";" //
+ type + ";" //
+ getCallableOverride(api) + ";" //
+ api.getName() + ";" //
+ "(" + parameterQualifiedTypeNamesToString(api) + ")" //
+ /* ext + */ ";" //
)
}

/**
* Holds for type `t` for fields that are relevant as an intermediate
* read or write step in the data flow analysis.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import shared.FlowSummaries

private class IncludeAllSummarizedCallable extends IncludeSummarizedCallable {
private class IncludeAllSummarizedCallable extends RelevantSummarizedCallable {
IncludeAllSummarizedCallable() { this instanceof SummarizedCallable }
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import shared.FlowSummaries
private import semmle.code.csharp.dataflow.ExternalFlow

class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
class IncludeFilteredSummarizedCallable extends RelevantSummarizedCallable {
IncludeFilteredSummarizedCallable() { this instanceof SummarizedCallable }

/**
Expand All @@ -13,7 +13,7 @@ class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
this.propagatesFlow(input, output, preservesValue) and
not exists(IncludeSummarizedCallable rsc |
not exists(RelevantSummarizedCallable rsc |
rsc.isBaseCallableOrPrototype() and
rsc.propagatesFlow(input, output, preservesValue) and
this.(UnboundCallable).overridesOrImplementsUnbound(rsc)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import semmle.code.csharp.frameworks.EntityFramework::EntityFramework
import shared.FlowSummaries

private class IncludeEFSummarizedCallable extends IncludeSummarizedCallable {
private class IncludeEFSummarizedCallable extends RelevantSummarizedCallable {
IncludeEFSummarizedCallable() { this instanceof EFSummarizedCallable }
}
66 changes: 31 additions & 35 deletions csharp/ql/test/shared/FlowSummaries.qll
Original file line number Diff line number Diff line change
@@ -1,44 +1,40 @@
import semmle.code.csharp.dataflow.FlowSummary
import semmle.code.csharp.dataflow.internal.FlowSummaryImpl::Private::TestOutput
import semmle.code.csharp.dataflow.internal.FlowSummaryImpl

abstract class IncludeSummarizedCallable extends RelevantSummarizedCallable {
IncludeSummarizedCallable() {
/** A flow summary to include in the `summary/3` query predicate. */
abstract class RelevantSummarizedCallable extends SummarizedCallable {
RelevantSummarizedCallable() {
[this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic()
}

/** Gets the qualified parameter types of this callable as a comma-separated string. */
private string parameterQualifiedTypeNamesToString() {
result =
concat(Parameter p, int i |
p = this.getParameter(i)
|
p.getType().getQualifiedName(), "," order by i
)
}

/** Holds if the summary should apply for all overrides of this. */
predicate isBaseCallableOrPrototype() {
this.getDeclaringType() instanceof Interface
or
exists(Modifiable m | m = [this.(Modifiable), this.(Accessor).getDeclaration()] |
m.isAbstract()
or
this.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
)
/** Holds if flow is propagated between `input` and `output`. */
predicate relevantSummary(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
this.propagatesFlow(input, output, preservesValue)
}
}

/** Gets a string representing whether the summary should apply for all overrides of this. */
private string getCallableOverride() {
if this.isBaseCallableOrPrototype() then result = "true" else result = "false"
}
/** Render the kind in the format used in flow summaries. */
private string renderKind(boolean preservesValue) {
preservesValue = true and result = "value"
or
preservesValue = false and result = "taint"
}

/** Gets a string representing the callable in semi-colon separated format for use in flow summaries. */
final override string getCallableCsv() {
exists(string namespace, string type |
this.getDeclaringType().hasQualifiedName(namespace, type) and
result =
namespace + ";" + type + ";" + this.getCallableOverride() + ";" + this.getName() + ";" + "("
+ this.parameterQualifiedTypeNamesToString() + ")"
)
}
/**
* A query predicate for outputting flow summaries in semi-colon separated format in QL tests.
* The syntax is: "namespace;type;overrides;name;signature;ext;inputspec;outputspec;kind",
* ext is hardcoded to empty.
*/
query predicate summary(string csv) {
exists(
RelevantSummarizedCallable c, SummaryComponentStack input, SummaryComponentStack output,
boolean preservesValue
|
c.relevantSummary(input, output, preservesValue) and
csv =
c.asPartialModel() + ";" + Public::getComponentStackCsv(input) + ";" +
Public::getComponentStackCsv(output) + ";" + renderKind(preservesValue)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -1022,46 +1022,6 @@ module Private {
}
}

/** Provides a query predicate for outputting a set of relevant flow summaries. */
module TestOutput {
/** A flow summary to include in the `summary/3` query predicate. */
abstract class RelevantSummarizedCallable extends SummarizedCallable {
/** Gets the string representation of this callable used by `summary/1`. */
abstract string getCallableCsv();

/** Holds if flow is propagated between `input` and `output`. */
predicate relevantSummary(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
this.propagatesFlow(input, output, preservesValue)
}
}

/** Render the kind in the format used in flow summaries. */
private string renderKind(boolean preservesValue) {
preservesValue = true and result = "value"
or
preservesValue = false and result = "taint"
}

/**
* A query predicate for outputting flow summaries in semi-colon separated format in QL tests.
* The syntax is: "namespace;type;overrides;name;signature;ext;inputspec;outputspec;kind",
* ext is hardcoded to empty.
*/
query predicate summary(string csv) {
exists(
RelevantSummarizedCallable c, SummaryComponentStack input, SummaryComponentStack output,
boolean preservesValue
|
c.relevantSummary(input, output, preservesValue) and
csv =
c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
getComponentStackCsv(output) + ";" + renderKind(preservesValue)
)
}
}

/**
* Provides query predicates for rendering the generated data flow graph for
* a summarized callable.
Expand Down
6 changes: 3 additions & 3 deletions java/ql/src/utils/model-generator/ModelGeneratorUtils.qll
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ predicate isRelevantContent(DataFlow::Content c) {
bindingset[input, output, kind]
string asSummaryModel(TargetApi api, string input, string output, string kind) {
result =
asPartialModel(api) + input + ";" //
api.asPartialModel() + input + ";" //
+ output + ";" //
+ kind
}
Expand All @@ -59,13 +59,13 @@ string asTaintModel(TargetApi api, string input, string output) {
*/
bindingset[input, kind]
string asSinkModel(TargetApi api, string input, string kind) {
result = asPartialModel(api) + input + ";" + kind
result = api.asPartialModel() + input + ";" + kind
}

/**
* Gets the source model for `api` with `output` and `kind`.
*/
bindingset[output, kind]
string asSourceModel(TargetApi api, string output, string kind) {
result = asPartialModel(api) + output + ";" + kind
result = api.asPartialModel() + output + ";" + kind
}
Loading