Rust: Value flow and taint flow through formatting strings#18394
Conversation
hvitved
left a comment
There was a problem hiding this comment.
LGTM, some small comments.
| result = e.(BreakExprCfgNode).getExpr() or | ||
| result = e.(BlockExprCfgNode).getTailExpr() or | ||
| result = e.(MatchExprCfgNode).getArmExpr(_) or | ||
| result = e.(MacroExprCfgNode).getMacroCall().(MacroCallCfgNode).getExpandedNode() or |
There was a problem hiding this comment.
Should this rather be
result = e.(MacroExprCfgNode).getMacroCall() or
result = e.(MacroCallCfgNode).getExpandedNode() or?
There was a problem hiding this comment.
Conceptually yes, but MacroCall is not an expression and doesn't have a node in the data-flow graph, so it won't work. To do that I think we'd have to change the type of getALastEvalNode and add a new kind of data-flow node for MacroCall.
ThaSo just adding a step over the MacroCall seems simpler and is also what we do for the other kind of nodes that have their expression nested further down.
We could add a method on MacroExprCfgNode to get the expanded node directly?
There was a problem hiding this comment.
Thanks, let's leave it as-is then.
| @@ -0,0 +1,2 @@ | |||
| edges | |||
There was a problem hiding this comment.
This file should be removed again.
| @@ -0,0 +1,122 @@ | |||
| localStep | |||
There was a problem hiding this comment.
Is the corresponding .ql test missing?
This PR adds:
format_args!to theformat_args!expression itself.format!.The original goal was to get taint through the
format!macro. But since its definition uses aletstatement, the issue in #18330 prevents that from working. Once that is fixed, the changes here should give flow throughfomat!.MacroCallAST nodes are now included in the CFG in post-order. Previously they where skipped over and not included in the CFG, but this caused thegetMacroCallpredicate onMacroExprCfgNodeto never have any results. Including them in the CFG fixes that, and I don't think there's any reason to exclude them.