Rust: Add basic skeleton setup for data flow #17871
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "source" | ||
| } | ||
|
|
||
| fn sink(s: &str) { |
Check notice
Code scanning / CodeQL
Unused variable Note test
paldepind
force-pushed
the
rust-data-flow-skeleton
branch
from
4f855c5 to
7217469
Compare
paldepind
force-pushed
the
rust-data-flow-skeleton
branch
from
7217469 to
44bebed
Compare
|
The qldoc CI job is failling with the errors below. I'm not sure why it's complaining about these modules, as they're preexisting modules? |
There was a problem hiding this comment.
LGTM, only some minor things. I think it would also be nice to enable the consistency checks, see e.g. https://github.com/github/codeql/blob/main/ruby/ql/consistency-queries/DataFlowConsistency.ql.
|
|
||
| private import rust | ||
| private import codeql.dataflow.DataFlow | ||
| private import internal.DataFlowImpl as DataFlowImpl |
There was a problem hiding this comment.
I don't think the as DataFlowImpl bit is needed.
There was a problem hiding this comment.
I added it for this line here.
There was a problem hiding this comment.
I see, that goes away with the other suggested changes 😄
| /** | ||
| * Gets the expression that corresponds to this node, if any. | ||
| */ | ||
| Expr asExpr() { none() } |
There was a problem hiding this comment.
The return type here should be CfgNode, because we will base data flow on the CFG and not on the AST.
There was a problem hiding this comment.
To me it seems a bit confusing for users and query writers if asExpr doesn't return an expression? There is also another predicate getCfgNode to get a CFG node.
From a quick grep it seems that in Swift, Go, Java, C++, and C# the asExpr predicate gives an expression. Only in Ruby does it give a CFG node.
There was a problem hiding this comment.
Let's leave it at Expr, for now, and then change it once we have a proper Expr layer on top of CfgNode.
| private module Cached { | ||
| cached | ||
| newtype TNode = | ||
| TExprNode(CfgNode n, Expr e) or |
There was a problem hiding this comment.
Is now
TExprNode(CfgNode n, Expr e) { n.getAstNode() = e } or
This PR adds the basic setup for data flow and taint tracking with the goal that