Merge pull request #10155 from MathiasVP/swift-properties-as-callables

Swift: Model property getters, setters and observers as callables

Author: MathiasVP

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

@@ -93,6 +93,20 @@ class CfgNode extends ControlFlowNode, TElementNode {
   final Split getASplit() { result = splits.getASplit() }
 }
 
+private Expr getAst(ControlFlowElement n) {
+  result = n.asAstNode()
+  or
+  result = n.(PropertyGetterElement).getRef()
+  or
+  result = n.(PropertySetterElement).getAssignExpr()
+  or
+  result = n.(PropertyObserverElement).getAssignExpr()
+  or
+  result = n.(ClosureElement).getAst()
+  or
+  result = n.(KeyPathElement).getAst()
+}
+
 /** A control-flow node that wraps an AST expression. */
 class ExprCfgNode extends CfgNode {
   Expr e;
@@ -108,21 +122,43 @@ class PropertyGetterCfgNode extends CfgNode {
   override PropertyGetterElement n;
 
   Expr getRef() { result = n.getRef() }
+
+  CfgNode getBase() { getAst(result.getNode()) = n.getBase() }
+
+  AccessorDecl getAccessorDecl() { result = n.getAccessorDecl() }
 }
 
 /** A control-flow node that wraps a property setter. */
 class PropertySetterCfgNode extends CfgNode {
   override PropertySetterElement n;
 
   AssignExpr getAssignExpr() { result = n.getAssignExpr() }
+
+  CfgNode getBase() { getAst(result.getNode()) = n.getBase() }
+
+  CfgNode getSource() { getAst(result.getNode()) = n.getAssignExpr().getSource() }
+
+  AccessorDecl getAccessorDecl() { result = n.getAccessorDecl() }
+}
+
+class PropertyObserverCfgNode extends CfgNode {
+  override PropertyObserverElement n;
+
+  AssignExpr getAssignExpr() { result = n.getAssignExpr() }
+
+  CfgNode getBase() { getAst(result.getNode()) = n.getBase() }
+
+  CfgNode getSource() { getAst(result.getNode()) = n.getAssignExpr().getSource() }
+
+  AccessorDecl getAccessorDecl() { result = n.getObserver() }
 }
 
 class ApplyExprCfgNode extends ExprCfgNode {
   override ApplyExpr e;
 
-  ExprCfgNode getArgument(int index) {
-    result.getNode().asAstNode() = e.getArgument(index).getExpr()
-  }
+  CfgNode getArgument(int index) { getAst(result.getNode()) = e.getArgument(index).getExpr() }
+
+  CfgNode getQualifier() { getAst(result.getNode()) = e.getQualifier() }
 
   AbstractFunctionDecl getStaticTarget() { result = e.getStaticTarget() }
 

swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll

@@ -123,6 +123,8 @@ class PropertyGetterElement extends ControlFlowElement, TPropertyGetterElement {
   Expr getRef() { result = ref }
 
   AccessorDecl getAccessorDecl() { result = accessor }
+
+  Expr getBase() { result = ref.(LookupExpr).getBase() }
 }
 
 class PropertySetterElement extends ControlFlowElement, TPropertySetterElement {
@@ -138,6 +140,8 @@ class PropertySetterElement extends ControlFlowElement, TPropertySetterElement {
   AccessorDecl getAccessorDecl() { result = accessor }
 
   AssignExpr getAssignExpr() { result = assign }
+
+  Expr getBase() { result = assign.getDest().(LookupExpr).getBase() }
 }
 
 class PropertyObserverElement extends ControlFlowElement, TPropertyObserverElement {
@@ -163,6 +167,8 @@ class PropertyObserverElement extends ControlFlowElement, TPropertyObserverEleme
   predicate isDidSet() { observer.isDidSet() }
 
   AssignExpr getAssignExpr() { result = assign }
+
+  Expr getBase() { result = assign.getDest().(LookupExpr).getBase() }
 }
 
 class FuncDeclElement extends ControlFlowElement, TFuncDeclElement {

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

@@ -69,6 +69,9 @@ class DataFlowCallable extends TDataFlowCallable {
 cached
 newtype TDataFlowCall =
   TNormalCall(ApplyExprCfgNode call) or
+  TPropertyGetterCall(PropertyGetterCfgNode getter) or
+  TPropertySetterCall(PropertySetterCfgNode setter) or
+  TPropertyObserverCall(PropertyObserverCfgNode obserer) or
   TSummaryCall(FlowSummaryImpl::Public::SummarizedCallable c, Node receiver) {
     FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
   }
@@ -84,6 +87,14 @@ class DataFlowCall extends TDataFlowCall {
   /** Gets the underlying source code call, if any. */
   ApplyExprCfgNode asCall() { none() }
 
+  /**
+   * Gets the i'th argument of call.class
+   * The qualifier is considered to have index `-1`.
+   */
+  CfgNode getArgument(int i) { none() }
+
+  final CfgNode getAnArgument() { result = this.getArgument(_) }
+
   /** Gets a textual representation of this call. */
   string toString() { none() }
 
@@ -111,13 +122,92 @@ private class NormalCall extends DataFlowCall, TNormalCall {
 
   override ApplyExprCfgNode asCall() { result = apply }
 
+  override CfgNode getArgument(int i) {
+    i = -1 and
+    result = apply.getQualifier()
+    or
+    result = apply.getArgument(i)
+  }
+
   override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(apply.getScope()) }
 
   override string toString() { result = apply.toString() }
 
   override Location getLocation() { result = apply.getLocation() }
 }
 
+class PropertyGetterCall extends DataFlowCall, TPropertyGetterCall {
+  private PropertyGetterCfgNode getter;
+
+  PropertyGetterCall() { this = TPropertyGetterCall(getter) }
+
+  override CfgNode getArgument(int i) {
+    i = -1 and
+    result = getter.getBase()
+  }
+
+  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(getter.getScope()) }
+
+  PropertyGetterCfgNode getGetter() { result = getter }
+
+  override string toString() { result = getter.toString() }
+
+  override Location getLocation() { result = getter.getLocation() }
+
+  AccessorDecl getAccessorDecl() { result = getter.getAccessorDecl() }
+}
+
+class PropertySetterCall extends DataFlowCall, TPropertySetterCall {
+  private PropertySetterCfgNode setter;
+
+  PropertySetterCall() { this = TPropertySetterCall(setter) }
+
+  override CfgNode getArgument(int i) {
+    i = -1 and
+    result = setter.getBase()
+    or
+    i = 0 and
+    result = setter.getSource()
+  }
+
+  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(setter.getScope()) }
+
+  PropertySetterCfgNode getSetter() { result = setter }
+
+  override string toString() { result = setter.toString() }
+
+  override Location getLocation() { result = setter.getLocation() }
+
+  AccessorDecl getAccessorDecl() { result = setter.getAccessorDecl() }
+}
+
+class PropertyObserverCall extends DataFlowCall, TPropertyObserverCall {
+  private PropertyObserverCfgNode observer;
+
+  PropertyObserverCall() { this = TPropertyObserverCall(observer) }
+
+  override CfgNode getArgument(int i) {
+    i = -1 and
+    result = observer.getBase()
+    or
+    // TODO: This is correct for `willSet` (which takes a `newValue` parameter),
+    // but for `didSet` (which takes an `oldValue` paramter) we need an rvalue
+    // for `getBase()`.
+    i = 0 and
+    result = observer.getSource()
+  }
+
+  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(observer.getScope()) }
+
+  PropertyObserverCfgNode getObserver() { result = observer }
+
+  override string toString() { result = observer.toString() }
+
+  override Location getLocation() { result = observer.getLocation() }
+
+  AccessorDecl getAccessorDecl() { result = observer.getAccessorDecl() }
+}
+
 class SummaryCall extends DataFlowCall, TSummaryCall {
   private FlowSummaryImpl::Public::SummarizedCallable c;
   private Node receiver;
@@ -145,6 +235,12 @@ private module Cached {
   cached
   DataFlowCallable viableCallable(DataFlowCall call) {
     result = TDataFlowFunc(call.asCall().getStaticTarget())
+    or
+    result = TDataFlowFunc(call.(PropertyGetterCall).getAccessorDecl())
+    or
+    result = TDataFlowFunc(call.(PropertySetterCall).getAccessorDecl())
+    or
+    result = TDataFlowFunc(call.(PropertyObserverCall).getAccessorDecl())
   }
 
   cached

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

@@ -154,7 +154,7 @@ private module ParameterNodes {
     predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
   }
 
-  class NormalParameterNode extends ParameterNodeImpl, SsaDefinitionNode {
+  class NormalParameterNode extends ParameterNodeImpl, SsaDefinitionNodeImpl {
     ParamDecl param;
 
     NormalParameterNode() {
@@ -169,10 +169,10 @@ private module ParameterNodes {
     override string toStringImpl() { result = param.toString() }
 
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      exists(Callable f, int index |
-        c = TDataFlowFunc(f) and
-        f.getParam(index) = param and
-        pos = TPositionalParameter(index)
+      exists(Callable f | c = TDataFlowFunc(f) |
+        exists(int index | f.getParam(index) = param and pos = TPositionalParameter(index))
+        or
+        f.getSelfParam() = param and pos = TThisParameter()
       )
     }
 
@@ -207,11 +207,68 @@ abstract class ArgumentNode extends Node {
 
 private module ArgumentNodes {
   class NormalArgumentNode extends ExprNode, ArgumentNode {
-    NormalArgumentNode() { exists(ApplyExpr call | call.getAnArgument().getExpr() = this.asExpr()) }
+    NormalArgumentNode() { exists(DataFlowCall call | call.getAnArgument() = this.getCfgNode()) }
 
     override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
-      call.asCall().getArgument(pos.(PositionalArgumentPosition).getIndex()).getExpr() =
-        this.asExpr()
+      call.getArgument(pos.(PositionalArgumentPosition).getIndex()) = this.getCfgNode()
+      or
+      pos = TThisArgument() and
+      call.getArgument(-1) = this.getCfgNode()
+    }
+  }
+
+  class PropertyGetterArgumentNode extends ExprNode, ArgumentNode {
+    private PropertyGetterCfgNode getter;
+
+    PropertyGetterArgumentNode() { getter.getBase() = this.getCfgNode() }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      call.(PropertyGetterCall).getGetter() = getter and
+      pos = TThisArgument()
+    }
+  }
+
+  class SetterArgumentNode extends ExprNode, ArgumentNode {
+    private PropertySetterCfgNode setter;
+
+    SetterArgumentNode() {
+      setter.getBase() = this.getCfgNode() or
+      setter.getSource() = this.getCfgNode()
+    }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      call.(PropertySetterCall).getSetter() = setter and
+      (
+        pos = TThisArgument() and
+        setter.getBase() = this.getCfgNode()
+        or
+        pos.(PositionalArgumentPosition).getIndex() = 0 and
+        setter.getSource() = this.getCfgNode()
+      )
+    }
+  }
+
+  class ObserverArgumentNode extends ExprNode, ArgumentNode {
+    private PropertyObserverCfgNode observer;
+
+    ObserverArgumentNode() {
+      observer.getBase() = this.getCfgNode()
+      or
+      // TODO: This should be an rvalue representing the `getBase` when
+      // `observer` a `didSet` observer.
+      observer.getSource() = this.getCfgNode()
+    }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      call.(PropertySetterCall).getSetter() = observer and
+      (
+        pos = TThisArgument() and
+        observer.getBase() = this.getCfgNode()
+        or
+        // TODO: See the comment above for `didSet` observers.
+        pos.(PositionalArgumentPosition).getIndex() = 0 and
+        observer.getSource() = this.getCfgNode()
+      )
     }
   }
 }