Merge pull request #9972 from MathiasVP/swift-taint-through-interpolated-strings

Swift: Taint through interpolated strings

Author: MathiasVP

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

@@ -96,21 +96,31 @@ module Stmts {
 
     override predicate propagatesAbnormal(ControlFlowElement node) { none() }
 
+    private predicate isBodyOfTapExpr() { any(TapExpr tap).getBody() = ast }
+
+    // Note: If the brace statement is the body of a `TapExpr`, the first element is the variable
+    // declaration (see https://github.com/apple/swift/blob/main/include/swift/AST/Expr.h#L848)
+    // that's initialized by the `TapExpr`. In `TapExprTree` we've already visited this declaration,
+    // along with its initializer. So we skip the first element here.
+    private AstNode getFirstElement() {
+      if this.isBodyOfTapExpr() then result = ast.getElement(1) else result = ast.getFirstElement()
+    }
+
     override predicate first(ControlFlowElement first) {
       this.firstInner(first)
       or
-      not exists(ast.getFirstElement()) and first.asAstNode() = ast
+      not exists(this.getFirstElement()) and first.asAstNode() = ast
     }
 
     override predicate last(ControlFlowElement last, Completion c) {
       this.lastInner(last, c)
       or
-      not exists(ast.getFirstElement()) and
+      not exists(this.getFirstElement()) and
       last.asAstNode() = ast and
       c instanceof SimpleCompletion
     }
 
-    predicate firstInner(ControlFlowElement first) { astFirst(ast.getFirstElement(), first) }
+    predicate firstInner(ControlFlowElement first) { astFirst(this.getFirstElement(), first) }
 
     /** Gets the body of the i'th `defer` statement. */
     private BraceStmt getDeferStmtBody(int i) {
@@ -1334,10 +1344,38 @@ module Exprs {
     override InterpolatedStringLiteralExpr ast;
 
     final override ControlFlowElement getChildElement(int i) {
-      none() // TODO
+      i = 0 and
+      result.asAstNode() = ast.getAppendingExpr().getFullyConverted()
+    }
+  }
+
+  /** Control-flow for a `TapExpr`. See the QLDoc for `TapExpr` for the semantics of a `TapExpr`. */
+  private class TapExprTree extends AstStandardPostOrderTree {
+    override TapExpr ast;
+
+    final override ControlFlowElement getChildElement(int i) {
+      // We first visit the local variable declaration.
+      i = 0 and
+      result.asAstNode() = ast.getVar()
+      or
+      // Then we visit the expression that gives the local variable its initial value.
+      i = 1 and
+      result.asAstNode() = ast.getSubExpr().getFullyConverted()
+      or
+      // And finally, we visit the body that potentially mutates the local variable.
+      // Note that the CFG for the body will skip the first element in the
+      // body because it's guarenteed to be the variable declaration
+      // that we've already visited at i = 0. See the explanation
+      // in `BraceStmtTree` for why this is necessary.
+      i = 2 and
+      result.asAstNode() = ast.getBody()
     }
   }
 
+  private class OpaqueValueExprTree extends AstLeafTree {
+    override OpaqueValueExpr ast;
+  }
+
   module DeclRefExprs {
     class DeclRefExprLValueTree extends AstLeafTree {
       override DeclRefExpr ast;

swift/ql/lib/codeql/swift/dataflow/Ssa.qll

@@ -80,7 +80,7 @@ module Ssa {
     cached
     predicate isInoutDef(ExprCfgNode argument) {
       exists(
-        CallExpr c, BasicBlock bb, int blockIndex, int argIndex, VarDecl v, InOutExpr argExpr // TODO: use CFG node for assignment expr
+        ApplyExpr c, BasicBlock bb, int blockIndex, int argIndex, VarDecl v, InOutExpr argExpr // TODO: use CFG node for assignment expr
       |
         this.definesAt(v, bb, blockIndex) and
         bb.getNode(blockIndex).getNode().asAstNode() = c and

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

@@ -64,10 +64,7 @@ private module Cached {
     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) {
-      param.isInout() and
-      call.getStaticTarget() = param.getDeclaringFunction()
-    } or
+    TInOutUpdateNode(Argument arg) { arg.getExpr() instanceof InOutExpr } or
     TSummaryNode(FlowSummary::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state)
 
   private predicate hasExprNode(CfgNode n, Expr e) {
@@ -202,7 +199,7 @@ abstract class ArgumentNode extends Node {
 
 private module ArgumentNodes {
   class NormalArgumentNode extends ExprNode, ArgumentNode {
-    NormalArgumentNode() { exists(CallExpr call | call.getAnArgument().getExpr() = this.asExpr()) }
+    NormalArgumentNode() { exists(ApplyExpr call | call.getAnArgument().getExpr() = this.asExpr()) }
 
     override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
       call.asCall().getArgument(pos.(PositionalArgumentPosition).getIndex()).getExpr() =
@@ -280,23 +277,22 @@ private module OutNodes {
   }
 
   class InOutUpdateNode extends OutNode, TInOutUpdateNode, NodeImpl {
-    ParamDecl param;
-    CallExpr call;
+    Argument arg;
 
-    InOutUpdateNode() { this = TInOutUpdateNode(param, call) }
+    InOutUpdateNode() { this = TInOutUpdateNode(arg) }
 
     override DataFlowCall getCall(ReturnKind kind) {
-      result.asCall().getExpr() = call and
-      kind.(ParamReturnKind).getIndex() = param.getIndex()
+      result.asCall().getExpr() = arg.getApplyExpr() and
+      kind.(ParamReturnKind).getIndex() = arg.getIndex()
     }
 
     override DataFlowCallable getEnclosingCallable() {
       result = this.getCall(_).getEnclosingCallable()
     }
 
-    override Location getLocationImpl() { result = call.getLocation() }
+    override Location getLocationImpl() { result = arg.getLocation() }
 
-    override string toStringImpl() { result = param.toString() }
+    override string toStringImpl() { result = arg.toString() }
   }
 }
 

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

@@ -29,7 +29,7 @@ predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain)
     certain = true
   )
   or
-  exists(CallExpr call, Argument arg |
+  exists(ApplyExpr call, Argument arg |
     arg.getExpr().(InOutExpr).getSubExpr() = v.getAnAccess() and
     call.getAnArgument() = arg and
     bb.getNode(i).getNode().asAstNode() = call and
@@ -39,6 +39,13 @@ predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain)
   v instanceof ParamDecl and
   bb.getNode(i).getNode().asAstNode() = v and
   certain = true
+  or
+  // Mark the subexpression as a write of the local variable declared in the `TapExpr`.
+  exists(TapExpr tap |
+    v = tap.getVar() and
+    bb.getNode(i).getNode().asAstNode() = tap.getSubExpr() and
+    certain = true
+  )
 }
 
 private predicate isLValue(DeclRefExpr ref) { any(AssignExpr assign).getDest() = ref }
@@ -58,4 +65,11 @@ predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain)
     bb.getScope() = func and
     certain = true
   )
+  or
+  // Mark the `TapExpr` as a read of the of the local variable.
+  exists(TapExpr tap |
+    v = tap.getVar() and
+    bb.getNode(i).getNode().asAstNode() = tap and
+    certain = true
+  )
 }

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

@@ -2,6 +2,8 @@ private import swift
 private import DataFlowPrivate
 private import TaintTrackingPublic
 private import codeql.swift.dataflow.DataFlow
+private import codeql.swift.dataflow.Ssa
+private import codeql.swift.controlflow.CfgNodes
 
 /**
  * Holds if `node` should be a sanitizer in all global taint flow configurations
@@ -16,7 +18,30 @@ private module Cached {
    * in all global taint flow configurations.
    */
   cached
-  predicate defaultAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { none() }
+  predicate defaultAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    // Flow through one argument of `appendLiteral` and `appendInterpolation` and to the second argument.
+    // This is needed for string interpolation generated by the compiler. An interpolated string
+    // like `"I am \(n) years old."` is represented as
+    // ```
+    // $interpolated = ""
+    // appendLiteral(&$interpolated, "I am ")
+    // appendInterpolation(&$interpolated, n)
+    // appendLiteral(&$interpolated, " years old.")
+    // ```
+    exists(ApplyExpr apply1, ApplyExpr apply2, ExprCfgNode e |
+      nodeFrom.asExpr() = [apply1, apply2].getAnArgument().getExpr() and
+      apply1.getFunction() = apply2 and
+      apply2.getStaticTarget().getName() = ["appendLiteral(_:)", "appendInterpolation(_:)"] and
+      e.getExpr() = apply2.getAnArgument().getExpr() and
+      nodeTo.asDefinition().(Ssa::WriteDefinition).isInoutDef(e)
+    )
+    or
+    // Flow from the computation of the interpolated string literal to the result of the interpolation.
+    exists(InterpolatedStringLiteralExpr interpolated |
+      nodeTo.asExpr() = interpolated and
+      nodeFrom.asExpr() = interpolated.getAppendingExpr()
+    )
+  }
 
   /**
    * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local

swift/ql/lib/codeql/swift/elements/expr/Argument.qll

@@ -5,4 +5,6 @@ class Argument extends ArgumentBase {
   override string toString() { result = this.getLabel() + ": " + this.getExpr().toString() }
 
   int getIndex() { any(ApplyExpr apply).getArgument(result) = this }
+
+  ApplyExpr getApplyExpr() { result.getAnArgument() = this }
 }

swift/ql/lib/codeql/swift/elements/expr/TapExpr.qll

@@ -1,4 +1,11 @@
-// generated by codegen/codegen.py, remove this comment if you wish to edit this file
 private import codeql.swift.generated.expr.TapExpr
 
+/**
+ * A `TapExpr` is an internal expression generated by the Swift compiler.
+ *
+ * If `e` is a `TapExpr`, the semantics of evaluating `e` is:
+ * 1. Create a local variable `e.getVar()` and assign it the value `e.getSubExpr()`.
+ * 2. Execute `e.getBody()` which potentially modifies the local variable.
+ * 3. Return the value of the local variable.
+ */
 class TapExpr extends TapExprBase { }