Skip to content

C#: Add post-update nodes for all argument nodes. #21383

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

Draft
michaelnebel wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

C#: Add post-update nodes for all argument nodes. #21383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0ce4e8
C#: Add data flow test for struct.
michaelnebel Feb 27, 2026
f34fd12
C#: Add struct source model example.
michaelnebel Feb 27, 2026
595fe70
C#: Remove the post update node restriction for argument and reverse …
michaelnebel Feb 27, 2026
a28bffc
C#: Do not clear content of collection type struct fields.
michaelnebel Feb 27, 2026
7d2b0b1
C#: Update test expected output for new tests.
michaelnebel Feb 27, 2026
6a3b56e
C#: Update other test expected output.
michaelnebel Feb 27, 2026
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
2 changes: 0 additions & 2 deletions csharp/ql/consistency-queries/DataFlowConsistency.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ private module Input implements InputSig<Location, CsharpDataFlow> {
n instanceof FlowSummaryNode
or
n instanceof ParamsArgumentNode
or
n.asExpr() = any(Expr e | not exprMayHavePostUpdateNode(e))
}

predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() }
Expand Down
31 changes: 8 additions & 23 deletions csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
private import FlowSummaryImpl as FlowSummaryImpl
private import semmle.code.csharp.dataflow.FlowSummary as FlowSummary
private import semmle.code.csharp.dataflow.internal.ExternalFlow
private import semmle.code.csharp.commons.Collections
private import semmle.code.csharp.Conversion
private import semmle.code.csharp.dataflow.internal.SsaImpl as SsaImpl
private import semmle.code.csharp.ExprOrStmtParent
Expand All @@ -16,7 +17,7 @@
private import semmle.code.csharp.frameworks.system.linq.Expressions
private import semmle.code.csharp.frameworks.NHibernate
private import semmle.code.csharp.frameworks.Razor
private import semmle.code.csharp.frameworks.system.Collections

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.csharp.commons.Collections
Loading
.
private import semmle.code.csharp.frameworks.system.threading.Tasks
private import semmle.code.csharp.internal.Location
private import codeql.util.Unit
Expand Down Expand Up @@ -1077,20 +1078,6 @@
}
}

/**
* Hold if `e` has a type that allows for it to have a post-update node.
*/
predicate exprMayHavePostUpdateNode(Expr e) {
exists(Type t | t = e.stripCasts().getType() |
t instanceof RefType and
not t instanceof NullType
or
t = any(TypeParameter tp | not tp.isValueType())
or
t.isRefLikeType()
)
}

/** A collection of cached types and predicates to be evaluated in the same stage. */
cached
private module Cached {
Expand Down Expand Up @@ -1137,15 +1124,12 @@
cfn.getAstNode().(ObjectCreation).hasInitializer()
} or
TExprPostUpdateNode(ControlFlow::Nodes::ExprNode cfn) {
(
cfn.getExpr() instanceof Argument
or
cfn =
LocalFlow::getPostUpdateReverseStep(any(ControlFlow::Nodes::ExprNode e |
exists(any(SourcePostUpdateNode p).getPreUpdateNode().asExprAtNode(e))
))
) and
exprMayHavePostUpdateNode(cfn.getExpr())
cfn.getExpr() instanceof Argument
or
cfn =
LocalFlow::getPostUpdateReverseStep(any(ControlFlow::Nodes::ExprNode e |
exists(any(SourcePostUpdateNode p).getPreUpdateNode().asExprAtNode(e))
))
or
exists(Expr e | e = cfn.getExpr() |
fieldOrPropertyStore(_, _, _, e, true)
Expand Down Expand Up @@ -2545,6 +2529,7 @@
a.getType() = s and
f = s.getAField() and
c.(FieldContent).getField() = f.getUnboundDeclaration() and
not f.getType() instanceof CollectionType and
not f.isRef()
)
or
Expand Down
2 changes: 2 additions & 0 deletions csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
| CSharp7.cs:198:22:198:35 | ref ... | CSharp7.cs:198:17:198:18 | access to local variable r4 |
| CSharp7.cs:198:26:198:35 | [post] this access | CSharp7.cs:199:9:199:18 | this access |
| CSharp7.cs:198:26:198:35 | this access | CSharp7.cs:199:9:199:18 | this access |
| CSharp7.cs:198:33:198:34 | [post] access to local variable r1 | CSharp7.cs:199:16:199:17 | access to local variable r1 |
| CSharp7.cs:198:33:198:34 | access to local variable r1 | CSharp7.cs:199:16:199:17 | access to local variable r1 |
| CSharp7.cs:199:22:199:22 | 3 | CSharp7.cs:199:9:199:22 | ... = ... |
| CSharp7.cs:202:24:202:24 | SSA param(p) | CSharp7.cs:205:20:205:20 | access to parameter p |
Expand Down Expand Up @@ -321,6 +322,7 @@
| CSharp7.cs:283:20:283:62 | call to method Select<KeyValuePair<Int32,String>,(Int32,String)> | CSharp7.cs:283:13:283:16 | access to local variable list |
| CSharp7.cs:283:32:283:35 | SSA param(item) | CSharp7.cs:283:41:283:44 | access to parameter item |
| CSharp7.cs:283:32:283:35 | item | CSharp7.cs:283:32:283:35 | SSA param(item) |
| CSharp7.cs:283:41:283:44 | [post] access to parameter item | CSharp7.cs:283:51:283:54 | access to parameter item |
| CSharp7.cs:283:41:283:44 | access to parameter item | CSharp7.cs:283:41:283:48 | access to property Key |
| CSharp7.cs:283:41:283:44 | access to parameter item | CSharp7.cs:283:51:283:54 | access to parameter item |
| CSharp7.cs:283:51:283:54 | access to parameter item | CSharp7.cs:283:51:283:60 | access to property Value |
Expand Down
6 changes: 6 additions & 0 deletions csharp/ql/test/library-tests/dataflow/external-models/Sources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ void Foo()

x = TaggedSrcPropertyGetter;
x = this[0];

S s;
StructSrc(s);
}

[SourceAttribute]
Expand Down Expand Up @@ -65,7 +68,10 @@ void SrcArg(object src) { }

[SourceAttribute]
object this[int i] => null;

void StructSrc(S s) { }
}

struct S { }
class SourceAttribute : System.Attribute { }
}
13 changes: 7 additions & 6 deletions csharp/ql/test/library-tests/dataflow/external-models/srcs.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ invalidModelRow
| Sources.cs:20:17:20:33 | call to method SrcTwoArg | local |
| Sources.cs:22:17:22:39 | access to property TaggedSrcPropertyGetter | local |
| Sources.cs:23:17:23:23 | access to indexer | local |
| Sources.cs:27:14:27:20 | this | local |
| Sources.cs:27:29:27:45 | taggedMethodParam | local |
| Sources.cs:31:47:31:60 | taggedSrcParam | local |
| Sources.cs:43:45:43:45 | p | local |
| Sources.cs:50:50:50:50 | p | local |
| Sources.cs:56:16:56:30 | this | local |
| Sources.cs:26:23:26:23 | [post] access to local variable s | local |
| Sources.cs:30:14:30:20 | this | local |
| Sources.cs:30:29:30:45 | taggedMethodParam | local |
| Sources.cs:34:47:34:60 | taggedSrcParam | local |
| Sources.cs:46:45:46:45 | p | local |
| Sources.cs:53:50:53:50 | p | local |
| Sources.cs:59:16:59:30 | this | local |
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/dataflow/external-models/srcs.ext.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ extensions:
- ["My.Qltest", "SourceAttribute", false, "", "", "Attribute", "", "local", "manual"]
- ["My.Qltest", "SourceAttribute", false, "", "", "Attribute.Getter", "ReturnValue", "local", "manual"]
- ["My.Qltest", "A", false, "SrcTwoArg", "(System.String,System.String)", "", "ReturnValue", "local", "manual"]
- ["My.Qltest", "A", false, "StructSrc", "", "", "Argument[0]", "local", "manual"]
Loading