hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-20 02:44:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4606 from hvitved/csharp/dataflow/ef

Browse Source 
C#: Precise data flow for EntityFramework(Core)
This commit is contained in:
Tom Hvitved
2020-11-10 15:54:20 +01:00
committed by GitHub
parent 26286e534e 697b0ba4c1
commit 7e89e57f5d
16 changed files with 1335 additions and 150 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
csharp/ql/src/semmle/code/csharp/dataflow/FlowSummary.qll
View File

@@ -8,6 +8,7 @@ private import internal.FlowSummarySpecific::Private
private import internal.DataFlowPublic as DataFlowPublic
// import all instances below
private import semmle.code.csharp.dataflow.LibraryTypeDataFlow
private import semmle.code.csharp.frameworks.EntityFramework
class SummarizableCallable = Impl::Public::SummarizableCallable;
@@ -135,6 +136,17 @@ module SummaryOutput {
result = TDelegateSummaryOutput(i, j) and
hasDelegateArgumentPosition2(c, i, j)
}
/**
* Gets an output specification that specifies the `output` of `target` as the
* output. That is, data will flow into one callable and out of another callable
* (`target`).
*
* `output` is limited to (this) parameters and ordinary returns.
*/
SummaryOutput jump(SummarizableCallable target, SummaryOutput output) {
result = TJumpSummaryOutput(target, toReturnKind(output))
}
}
class SummarizedCallable = Impl::Public::SummarizedCallable;

3
csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll
View File

@@ -21,6 +21,9 @@ DotNet::Callable getCallableForDataFlow(DotNet::Callable c) {
result = sourceDecl and
result instanceof SummarizedCallable
or
result = sourceDecl and
FlowSummaryImpl::Private::summary(_, _, _, SummaryOutput::jump(result, _), _, _)
or
result.hasBody() and
if sourceDecl.getFile().fromSource()
then

64
csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View File

@@ -469,12 +469,9 @@ private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
private predicate fieldOrPropertyRead(Expr e1, Content c, FieldOrPropertyRead e2) {
e1 = e2.getQualifier() and
exists(FieldOrProperty ret | c = ret.getContent() |
ret.isFieldLike() and
ret = e2.getTarget()
or
exists(ContentList cl, Property target |
FlowSummaryImpl::Private::summary(_, _, _, _, cl, _) and
cl.contains(ret.getContent()) and
exists(Property target |
target.getGetter() = e2.(PropertyCall).getARuntimeTarget() and
overridesOrImplementsSourceDecl(target, ret)
)
@@ -640,6 +637,10 @@ private module Cached {
output = SummaryOutput::delegate(delegateIndex, parameterIndex)
)
} or
TSummaryJumpNode(SummarizedCallable c, SummarizableCallable target, ReturnKind rk) {
FlowSummaryImpl::Private::summary(c, _, _,
FlowSummarySpecific::Private::TJumpSummaryOutput(target, rk), _, _)
} or
TParamsArgumentNode(ControlFlow::Node callCfn) {
callCfn = any(Call c | isParamsArg(c, _, _)).getAControlFlowNode()
}
@@ -685,8 +686,19 @@ private module Cached {
* taken into account.
*/
cached
predicate jumpStepImpl(ExprNode pred, ExprNode succ) {
predicate jumpStepImpl(Node pred, Node succ) {
pred.(NonLocalJumpNode).getAJumpSuccessor(true) = succ
or
exists(FieldOrProperty fl, FieldOrPropertyRead flr |
fl.isStatic() and
fl.isFieldLike() and
fl.getAnAssignedValue() = pred.asExpr() and
fl.getAnAccess() = flr and
flr = succ.asExpr() and
flr.hasNonlocalValue()
)
or
succ = pred.(SummaryJumpNode).getAJumpTarget()
}
cached
@@ -1613,6 +1625,28 @@ private class SummaryInternalNode extends SummaryNodeImpl, TSummaryInternalNode
override string toStringImpl() { result = "[summary] " + state + " in " + c }
}
/** A data-flow node used to model flow summaries with jumps. */
private class SummaryJumpNode extends SummaryNodeImpl, TSummaryJumpNode {
private SummarizedCallable c;
private SummarizableCallable target;
private ReturnKind rk;
SummaryJumpNode() { this = TSummaryJumpNode(c, target, rk) }
/** Gets a jump target of this node. */
OutNode getAJumpTarget() { target = viableCallable(result.getCall(rk)) }
override Callable getEnclosingCallableImpl() { result = c }
override DotNet::Type getTypeImpl() { result = target.getReturnType() }
override ControlFlow::Node getControlFlowNodeImpl() { none() }
override Location getLocationImpl() { result = c.getLocation() }
override string toStringImpl() { result = "[summary] jump to " + target }
}
/** A field or a property. */
class FieldOrProperty extends Assignable, Modifiable {
FieldOrProperty() {
@@ -1669,26 +1703,6 @@ private class FieldOrPropertyRead extends FieldOrPropertyAccess, AssignableRead
}
}
/** A write to a static field/property. */
private class StaticFieldLikeJumpNode extends NonLocalJumpNode, ExprNode {
FieldOrProperty fl;
FieldOrPropertyRead flr;
ExprNode succ;
StaticFieldLikeJumpNode() {
fl.isStatic() and
fl.isFieldLike() and
fl.getAnAssignedValue() = this.getExpr() and
fl.getAnAccess() = flr and
flr = succ.getExpr() and
flr.hasNonlocalValue()
}
override ExprNode getAJumpSuccessor(boolean preservesValue) {
result = succ and preservesValue = true
}
}
predicate jumpStep = jumpStepImpl/2;
private class StoreStepConfiguration extends ControlFlowReachabilityConfiguration {

26
csharp/ql/src/semmle/code/csharp/dataflow/internal/FlowSummarySpecific.qll
View File

@@ -4,13 +4,13 @@
private import csharp
private import semmle.code.csharp.frameworks.system.linq.Expressions
private import DataFlowDispatch
module Private {
private import Public
private import DataFlowPrivate as DataFlowPrivate
private import DataFlowPublic as DataFlowPublic
private import FlowSummaryImpl as Impl
private import DataFlowDispatch
private import semmle.code.csharp.Unification
class Content = DataFlowPublic::Content;
@@ -56,7 +56,19 @@ module Private {
TParameterSummaryOutput(int i) {
i in [-1, any(SummarizableCallable c).getAParameter().getPosition()]
} or
TDelegateSummaryOutput(int i, int j) { hasDelegateArgumentPosition2(_, i, j) }
TDelegateSummaryOutput(int i, int j) { hasDelegateArgumentPosition2(_, i, j) } or
TJumpSummaryOutput(SummarizableCallable target, ReturnKind rk) {
rk instanceof NormalReturnKind and
(
target instanceof Constructor or
not target.getReturnType() instanceof VoidType
)
or
rk instanceof QualifierReturnKind and
not target.(Modifiable).isStatic()
or
exists(target.getParameter(rk.(OutRefReturnKind).getPosition()))
}
/** Gets the return kind that matches `sink`, if any. */
ReturnKind toReturnKind(SummaryOutput output) {
@@ -92,6 +104,11 @@ module Private {
output = TDelegateSummaryOutput(i, j) and
result = DataFlowPrivate::TSummaryDelegateArgumentNode(c, i, j)
)
or
exists(SummarizableCallable target, ReturnKind rk |
output = TJumpSummaryOutput(target, rk) and
result = DataFlowPrivate::TSummaryJumpNode(c, target, rk)
)
}
/** Gets the internal summary node for the given values. */
@@ -151,6 +168,11 @@ module Public {
this = TDelegateSummaryOutput(delegateIndex, parameterIndex) and
result = "parameter " + parameterIndex + " of delegate parameter " + delegateIndex
)
or
exists(SummarizableCallable target, ReturnKind rk |
this = TJumpSummaryOutput(target, rk) and
result = "jump to " + target + " (" + rk + ")"
)
}
}
}

279
csharp/ql/src/semmle/code/csharp/frameworks/EntityFramework.qll
View File

@@ -3,17 +3,19 @@
*/
import csharp
private import DataFlow
private import semmle.code.csharp.frameworks.System
private import semmle.code.csharp.frameworks.system.data.Entity
private import semmle.code.csharp.frameworks.system.collections.Generic
private import semmle.code.csharp.frameworks.Sql
private import semmle.code.csharp.dataflow.LibraryTypeDataFlow
private import semmle.code.csharp.dataflow.FlowSummary
/**
* Definitions relating to the `System.ComponentModel.DataAnnotations`
* namespace.
*/
module DataAnnotations {
/** Class for `NotMappedAttribute`. */
/** The `NotMappedAttribute` attribute. */
class NotMappedAttribute extends Attribute {
NotMappedAttribute() {
this
@@ -23,6 +25,11 @@ module DataAnnotations {
}
}
/** Holds if `a` has the `[NotMapped]` attribute */
private predicate isNotMapped(Attributable a) {
a.getAnAttribute() instanceof DataAnnotations::NotMappedAttribute
}
/**
* Definitions relating to the `Microsoft.EntityFrameworkCore` or
* `System.Data.Entity` namespaces.
@@ -66,6 +73,41 @@ module EntityFramework {
/** The class `Microsoft.EntityFrameworkCore.DbSet<>` or `System.Data.Entity.DbSet<>`. */
class DbSet extends EFClass, UnboundGenericClass {
DbSet() { this.getName() = "DbSet<>" }
/** Gets a method that adds or updates entities in a DB set. */
SummarizableMethod getAnAddOrUpdateMethod(boolean range) {
exists(string name | result = this.getAMethod(name) |
name in ["Add", "AddAsync", "Attach", "Update"] and
range = false
or
name in ["AddRange", "AddRangeAsync", "AttachRange", "UpdateRange"] and
range = true
)
}
}
/** A flow summary for EntityFramework. */
abstract class EFSummarizedCallable extends SummarizedCallable { }
private class DbSetAddOrUpdate extends EFSummarizedCallable {
private boolean range;
DbSetAddOrUpdate() { this = any(DbSet c).getAnAddOrUpdateMethod(range) }
override predicate propagatesFlow(
SummaryInput input, ContentList inputContents, SummaryOutput output,
ContentList outputContents, boolean preservesValue
) {
input = SummaryInput::parameter(0) and
(
if range = true
then inputContents = ContentList::element()
else inputContents = ContentList::empty()
) and
output = SummaryOutput::thisParameter() and
outputContents = ContentList::element() and
preservesValue = true
}
}
/** The class `Microsoft.EntityFrameworkCore.DbQuery<>` or `System.Data.Entity.DbQuery<>`. */
@@ -107,29 +149,14 @@ module EntityFramework {
MappedProperty() {
this = any(MappedType t).getAMember() and
this.isPublic() and
not this.getAnAttribute() instanceof DataAnnotations::NotMappedAttribute
not isNotMapped(this)
}
}
/** The struct `Microsoft.EntityFrameworkCore.RawSqlString`. */
class RawSqlStringStruct extends Struct, LibraryTypeDataFlow {
private class RawSqlStringStruct extends Struct {
RawSqlStringStruct() { this.getQualifiedName() = "Microsoft.EntityFrameworkCore.RawSqlString" }
override predicate callableFlow(
CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c,
boolean preservesValue
) {
c = this.getAConstructor() and
source.(CallableFlowSourceArg).getArgumentIndex() = 0 and
sink instanceof CallableFlowSinkReturn and
preservesValue = false
or
c = this.getAConversionTo() and
source.(CallableFlowSourceArg).getArgumentIndex() = 0 and
sink instanceof CallableFlowSinkReturn and
preservesValue = false
}
/** Gets a conversion operator from `string` to `RawSqlString`. */
ConversionOperator getAConversionTo() {
result = this.getAMember() and
@@ -138,6 +165,35 @@ module EntityFramework {
}
}
private class RawSqlStringSummarizedCallable extends EFSummarizedCallable {
private SummaryInput input_;
private SummaryOutput output_;
private boolean preservesValue_;
RawSqlStringSummarizedCallable() {
exists(RawSqlStringStruct s |
this = s.getAConstructor() and
input_ = SummaryInput::parameter(0) and
this.getNumberOfParameters() > 0 and
output_ = SummaryOutput::return() and
preservesValue_ = false
or
this = s.getAConversionTo() and
input_ = SummaryInput::parameter(0) and
output_ = SummaryOutput::return() and
preservesValue_ = false
)
}
override predicate propagatesFlow(
SummaryInput input, SummaryOutput output, boolean preservesValue
) {
input = input_ and
output = output_ and
preservesValue = preservesValue_
}
}
/**
* A parameter that accepts raw SQL. Parameters of type `System.FormattableString`
* are not included as they are not vulnerable to SQL injection.
@@ -192,18 +248,183 @@ module EntityFramework {
override Expr getSql() { result = this.getArgumentForName("sql") }
}
/**
* A dataflow node whereby data flows from a property write to a property read
* via some database. The assumption is that all writes can flow to all reads.
*/
class MappedPropertyJumpNode extends DataFlow::NonLocalJumpNode {
MappedProperty property;
/** Holds if `t` is compatible with a DB column type. */
private predicate isColumnType(Type t) {
t instanceof SimpleType
or
t instanceof StringType
or
t instanceof Enum
or
t instanceof SystemDateTimeStruct
or
isColumnType(t.(NullableType).getUnderlyingType())
}
MappedPropertyJumpNode() { this.asExpr() = property.getAnAssignedValue() }
/** A DB Context. */
private class DbContextClass extends Class {
DbContextClass() { this.getBaseClass*().getSourceDeclaration() instanceof DbContext }
override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
result.asExpr().(PropertyRead).getTarget() = property and
preservesValue = false
/**
* Gets a `DbSet<elementType>` property belonging to this DB context.
*
* For example `Persons` with `elementType = Person` in
*
* ```csharp
* class MyContext : DbContext
* {
* public virtual DbSet<Person> Persons { get; set; }
* public virtual DbSet<Address> Addresses { get; set; }
* }
* ```
*/
private Property getADbSetProperty(Class elementType) {
exists(ConstructedClass c |
result.getType() = c and
c.getSourceDeclaration() instanceof DbSet and
elementType = c.getTypeArgument(0) and
this.hasMember(any(Property p | result = p.getSourceDeclaration())) and
not isNotMapped([result.(Attributable), elementType])
)
}
/**
* Holds if `[c2, c1]` is part of a valid access path starting from a `DbSet<T>`
* property belonging to this DB context. `t1` is the type of `c1` and `t2` is
* the type of `c2`.
*
* If `t2` is a column type, `c2` will be included in the model (see
* https://docs.microsoft.com/en-us/ef/core/modeling/entity-types?tabs=data-annotations).
*/
private predicate step(Content c1, Type t1, Content c2, Type t2) {
exists(Property p1 |
p1 = this.getADbSetProperty(t2) and
c1.(PropertyContent).getProperty() = p1 and
t1 = p1.getType() and
c2 instanceof ElementContent
)
or
step(_, _, c1, t1) and
not isNotMapped(t2) and
(
// Navigation property (https://docs.microsoft.com/en-us/ef/ef6/fundamentals/relationships)
exists(Property p2 |
p2.getDeclaringType().(Class) = t1 and
not isColumnType(t1) and
c2.(PropertyContent).getProperty() = p2 and
t2 = p2.getType() and
not isNotMapped(p2)
)
or
exists(ConstructedInterface ci |
c1 instanceof PropertyContent and
t1.(ValueOrRefType).getABaseType*() = ci and
not t1 instanceof StringType and
ci.getSourceDeclaration() instanceof SystemCollectionsGenericIEnumerableTInterface and
c2 instanceof ElementContent and
t2 = ci.getTypeArgument(0)
)
)
}
/**
* Gets a property belonging to the model of this DB context, which is mapped
* directly to a column in the underlying DB.
*
* For example the `Name` and `Id` properties of `Person`, but not `Title`
* as it is explicitly unmapped, in
*
* ```csharp
* class Person
* {
* public int Id { get; set; }
* public string Name { get; set; }
*
* [NotMapped]
* public string Title { get; set; }
* }
*
* class MyContext : DbContext
* {
* public virtual DbSet<Person> Persons { get; set; }
* public virtual DbSet<Address> Addresses { get; set; }
* }
* ```
*/
private Property getAColumnProperty() {
exists(PropertyContent c, Type t |
this.step(_, _, c, t) and
c.getProperty() = result and
isColumnType(t)
)
}
/** Gets a `SaveChanges[Async]` method. */
pragma[nomagic]
SummarizableMethod getASaveChanges() {
this.hasMethod(result) and
result.getName().matches("SaveChanges%")
}
/** Holds if content list `head :: tail` is required. */
predicate requiresContentList(
Content head, Type headType, ContentList tail, Type tailType, Property last
) {
exists(PropertyContent p |
last = this.getAColumnProperty() and
p.getProperty() = last and
tail = ContentList::singleton(p) and
this.step(head, headType, p, tailType)
)
or
exists(Content tailHead, ContentList tailTail |
this.requiresContentList(tailHead, tailType, tailTail, _, last) and
tail = ContentList::cons(tailHead, tailTail) and
this.step(head, headType, tailHead, tailType)
)
}
/**
* Holds if the access path obtained by concatenating `head` onto `tail`
* is a path from `dbSet` (which is a `DbSet<T>` property belonging to
* this DB context) to `last`, which is a property that is mapped directly
* to a column in the underlying DB.
*/
pragma[noinline]
predicate pathFromDbSetToDbProperty(
Property dbSet, PropertyContent head, ContentList tail, Property last
) {
this.requiresContentList(head, _, tail, _, last) and
head.getProperty() = dbSet and
dbSet = this.getADbSetProperty(_)
}
}
private class DbContextSaveChanges extends EFSummarizedCallable {
private DbContextClass c;
DbContextSaveChanges() { this = c.getASaveChanges() }
override predicate requiresContentList(Content head, ContentList tail) {
c.requiresContentList(head, _, tail, _, _)
}
override predicate propagatesFlow(
SummaryInput input, ContentList inputContents, SummaryOutput output,
ContentList outputContents, boolean preservesValue
) {
exists(Property mapped |
preservesValue = true and
exists(PropertyContent sourceHead, ContentList sourceTail |
input = SummaryInput::thisParameter() and
c.pathFromDbSetToDbProperty(_, sourceHead, sourceTail, mapped) and
inputContents = ContentList::cons(sourceHead, sourceTail)
) and
exists(Property dbSetProp |
output = SummaryOutput::jump(dbSetProp.getGetter(), SummaryOutput::return()) and
c.pathFromDbSetToDbProperty(dbSetProp, _, outputContents, mapped)
)
)
}
}
}

5
csharp/ql/src/semmle/code/csharp/frameworks/System.qll
View File

@@ -729,3 +729,8 @@ class SystemGuid extends SystemStruct {
class SystemNotImplementedExceptionClass extends SystemClass {
SystemNotImplementedExceptionClass() { this.hasName("NotImplementedException") }
}
/** The `System.DateTime` struct. */
class SystemDateTimeStruct extends SystemStruct {
SystemDateTimeStruct() { this.hasName("DateTime") }
}