Merge pull request #9668 from MathiasVP/expr-nodes-for-properties

Swift: Make sure property setters and getters also have `ExprNodes`

Author: Robert Marsh

swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll

@@ -214,15 +214,15 @@ private module JoinBlockPredecessors {
   }
 
   int getId(JoinBlockPredecessor jbp) {
-    idOf(projctToAst(jbp.getFirstNode().(AstCfgNode).getNode()), result)
+    idOf(projctToAst(jbp.getFirstNode().(CfgNode).getNode()), result)
     or
     idOf(jbp.(EntryBasicBlock).getScope(), result)
   }
 
   string getSplitString(JoinBlockPredecessor jbp) {
-    result = jbp.getFirstNode().(AstCfgNode).getSplitsString()
+    result = jbp.getFirstNode().(CfgNode).getSplitsString()
     or
-    not exists(jbp.getFirstNode().(AstCfgNode).getSplitsString()) and
+    not exists(jbp.getFirstNode().(CfgNode).getSplitsString()) and
     result = ""
   }
 }

swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll

@@ -61,15 +61,15 @@ class ExitNode extends ControlFlowNode, TExitNode {
 /**
  * A node for an AST node.
  *
- * Each AST node maps to zero or more `AstCfgNode`s: zero when the node is unreachable
+ * Each AST node maps to zero or more `CfgNode`s: zero when the node is unreachable
  * (dead) code or not important for control flow, and multiple when there are different
  * splits for the AST node.
  */
-class AstCfgNode extends ControlFlowNode, TElementNode {
+class CfgNode extends ControlFlowNode, TElementNode {
   private Splits splits;
-  private ControlFlowElement n;
+  ControlFlowElement n;
 
-  AstCfgNode() { this = TElementNode(_, n, splits) }
+  CfgNode() { this = TElementNode(_, n, splits) }
 
   final override ControlFlowElement getNode() { result = n }
 
@@ -94,7 +94,7 @@ class AstCfgNode extends ControlFlowNode, TElementNode {
 }
 
 /** A control-flow node that wraps an AST expression. */
-class ExprCfgNode extends AstCfgNode {
+class ExprCfgNode extends CfgNode {
   Expr e;
 
   ExprCfgNode() { e = this.getNode().asAstNode() }
@@ -103,6 +103,20 @@ class ExprCfgNode extends AstCfgNode {
   Expr getExpr() { result = e }
 }
 
+/** A control-flow node that wraps a property getter. */
+class PropertyGetterCfgNode extends CfgNode {
+  override PropertyGetterElement n;
+
+  Expr getRef() { result = n.getRef() }
+}
+
+/** A control-flow node that wraps a property setter. */
+class PropertySetterCfgNode extends CfgNode {
+  override PropertySetterElement n;
+
+  AssignExpr getAssignExpr() { result = n.getAssignExpr() }
+}
+
 class ApplyExprCfgNode extends ExprCfgNode {
   override ApplyExpr e;
 

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

@@ -37,7 +37,7 @@ private class ExprNodeImpl extends ExprNode, NodeImpl {
 
   override string toStringImpl() { result = expr.toString() }
 
-  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(expr.getScope()) }
+  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(n.getScope()) }
 }
 
 private class SsaDefinitionNodeImpl extends SsaDefinitionNode, NodeImpl {
@@ -62,7 +62,7 @@ cached
 private module Cached {
   cached
   newtype TNode =
-    TExprNode(ExprCfgNode e) or
+    TExprNode(CfgNode n, Expr e) { hasExprNode(n, e) } or
     TSsaDefinitionNode(Ssa::Definition def) or
     TInoutReturnNode(ParamDecl param) { param.isInout() } or
     TInOutUpdateNode(ParamDecl param, CallExpr call) {
@@ -71,6 +71,14 @@ private module Cached {
     } or
     TSummaryNode(FlowSummary::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state)
 
+  private predicate hasExprNode(CfgNode n, Expr e) {
+    n.(ExprCfgNode).getExpr() = e
+    or
+    n.(PropertyGetterCfgNode).getRef() = e
+    or
+    n.(PropertySetterCfgNode).getAssignExpr() = e
+  }
+
   private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
     def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
     (

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll

@@ -56,13 +56,14 @@ class Node extends TNode {
  * `ControlFlow::Node`s.
  */
 class ExprNode extends Node, TExprNode {
-  ExprCfgNode expr;
+  CfgNode n;
+  Expr expr;
 
-  ExprNode() { this = TExprNode(expr) }
+  ExprNode() { this = TExprNode(n, expr) }
 
-  override Expr asExpr() { result = expr.getNode().asAstNode() }
+  override Expr asExpr() { result = expr }
 
-  override ControlFlowNode getCfgNode() { result = expr }
+  override ControlFlowNode getCfgNode() { result = n }
 }
 
 /**