Merge remote-tracking branch 'origin/main' into external-control-file-path

2026-05-05 13:45:19 +02:00 · 2021-10-21 10:58:38 +01:00
parent 16c62d22de f9f0fab0d0
commit 5734f51792
339 changed files with 8031 additions and 6720 deletions
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll
@@ -110,12 +110,12 @@ abstract class Configuration extends string {
  /**
   * Holds if data may flow from some source to `sink` for this configuration.
   */
-  predicate hasFlowTo(Node sink) { hasFlow(_, sink) }
+  predicate hasFlowTo(Node sink) { this.hasFlow(_, sink) }

  /**
   * Holds if data may flow from some source to `sink` for this configuration.
   */
-  predicate hasFlowToExpr(DataFlowExpr sink) { hasFlowTo(exprNode(sink)) }
+  predicate hasFlowToExpr(DataFlowExpr sink) { this.hasFlowTo(exprNode(sink)) }

  /**
   * Gets the exploration limit for `hasPartialFlow` and `hasPartialFlowRev`
@@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx {
  }

  int getPosition() { this.isParameterOf(_, result) }
+
+  predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) }
 }

 private class RetNodeEx extends NodeEx {
@@ -744,8 +746,12 @@ private module Stage1 {
      returnFlowCallableNodeCand(c, kind, config) and
      p.getEnclosingCallable() = c and
      exists(ap) and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition()
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition()
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -1394,8 +1400,12 @@ private module Stage2 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2083,8 +2093,12 @@ private module Stage3 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2843,8 +2857,12 @@ private module Stage4 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome {

  int getParameterPos() { p.isParameterOf(_, result) }

+  ParamNodeEx getParamNode() { result = p }
+
  override string toString() { result = p + ": " + ap }

  predicate hasLocationInfo(
@@ -3170,7 +3190,7 @@ private class AccessPathCons extends AccessPath, TAccessPathCons {
  }

  override string toString() {
-    result = "[" + this.toStringImpl(true) + length().toString() + ")]"
+    result = "[" + this.toStringImpl(true) + this.length().toString() + ")]"
    or
    result = "[" + this.toStringImpl(false)
  }
@@ -3309,9 +3329,11 @@ abstract private class PathNodeImpl extends PathNode {
    result = " <" + this.(PathNodeMid).getCallContext().toString() + ">"
  }

-  override string toString() { result = this.getNodeEx().toString() + ppAp() }
+  override string toString() { result = this.getNodeEx().toString() + this.ppAp() }

-  override string toStringWithContext() { result = this.getNodeEx().toString() + ppAp() + ppCtx() }
+  override string toStringWithContext() {
+    result = this.getNodeEx().toString() + this.ppAp() + this.ppCtx()
+  }

  override predicate hasLocationInfo(
    string filepath, int startline, int startcolumn, int endline, int endcolumn
@@ -3379,11 +3401,11 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {

  override PathNodeImpl getASuccessorImpl() {
    // an intermediate step to another intermediate node
-    result = getSuccMid()
+    result = this.getSuccMid()
    or
    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
    exists(PathNodeMid mid, PathNodeSink sink |
-      mid = getSuccMid() and
+      mid = this.getSuccMid() and
      mid.getNodeEx() = sink.getNodeEx() and
      mid.getAp() instanceof AccessPathNil and
      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
@@ -3460,7 +3482,7 @@ private predicate pathStep(
  exists(TypedContent tc | pathReadStep(mid, node, ap.push(tc), tc, cc)) and
  sc = mid.getSummaryCtx()
  or
-  pathIntoCallable(mid, node, _, cc, sc, _) and ap = mid.getAp()
+  pathIntoCallable(mid, node, _, cc, sc, _, _) and ap = mid.getAp()
  or
  pathOutOfCallable(mid, node, cc) and ap = mid.getAp() and sc instanceof SummaryCtxNone
  or
@@ -3537,14 +3559,16 @@ private predicate pathOutOfCallable(PathNodeMid mid, NodeEx out, CallContext cc)
 */
 pragma[noinline]
 private predicate pathIntoArg(
-  PathNodeMid mid, int i, CallContext cc, DataFlowCall call, AccessPath ap, AccessPathApprox apa
+  PathNodeMid mid, int i, CallContext cc, DataFlowCall call, AccessPath ap, AccessPathApprox apa,
+  Configuration config
 ) {
  exists(ArgNode arg |
    arg = mid.getNodeEx().asNode() and
    cc = mid.getCallContext() and
    arg.argumentOf(call, i) and
    ap = mid.getAp() and
-    apa = ap.getApprox()
+    apa = ap.getApprox() and
+    config = mid.getConfiguration()
  )
 }

@@ -3561,12 +3585,14 @@ private predicate parameterCand(
 pragma[nomagic]
 private predicate pathIntoCallable0(
  PathNodeMid mid, DataFlowCallable callable, int i, CallContext outercc, DataFlowCall call,
-  AccessPath ap
+  AccessPath ap, Configuration config
 ) {
  exists(AccessPathApprox apa |
-    pathIntoArg(mid, i, outercc, call, ap, apa) and
+    pathIntoArg(mid, pragma[only_bind_into](i), outercc, call, ap, pragma[only_bind_into](apa),
+      pragma[only_bind_into](config)) and
    callable = resolveCall(call, outercc) and
-    parameterCand(callable, any(int j | j <= i and j >= i), apa, mid.getConfiguration())
+    parameterCand(callable, pragma[only_bind_into](i), pragma[only_bind_into](apa),
+      pragma[only_bind_into](config))
  )
 }

@@ -3575,12 +3601,13 @@ private predicate pathIntoCallable0(
 * before and after entering the callable are `outercc` and `innercc`,
 * respectively.
 */
+pragma[nomagic]
 private predicate pathIntoCallable(
  PathNodeMid mid, ParamNodeEx p, CallContext outercc, CallContextCall innercc, SummaryCtx sc,
-  DataFlowCall call
+  DataFlowCall call, Configuration config
 ) {
  exists(int i, DataFlowCallable callable, AccessPath ap |
-    pathIntoCallable0(mid, callable, i, outercc, call, ap) and
+    pathIntoCallable0(mid, callable, i, outercc, call, ap, config) and
    p.isParameterOf(callable, i) and
    (
      sc = TSummaryCtxSome(p, ap)
@@ -3610,18 +3637,23 @@ private predicate paramFlowsThrough(
    ap = mid.getAp() and
    apa = ap.getApprox() and
    pos = sc.getParameterPos() and
-    not kind.(ParamUpdateReturnKind).getPosition() = pos
+    // we don't expect a parameter to return stored in itself, unless explicitly allowed
+    (
+      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      or
+      sc.getParamNode().allowParameterReturnInSelf()
+    )
  )
 }

 pragma[nomagic]
 private predicate pathThroughCallable0(
  DataFlowCall call, PathNodeMid mid, ReturnKindExt kind, CallContext cc, AccessPath ap,
-  AccessPathApprox apa
+  AccessPathApprox apa, Configuration config
 ) {
  exists(CallContext innercc, SummaryCtx sc |
-    pathIntoCallable(mid, _, cc, innercc, sc, call) and
-    paramFlowsThrough(kind, innercc, sc, ap, apa, unbindConf(mid.getConfiguration()))
+    pathIntoCallable(mid, _, cc, innercc, sc, call, config) and
+    paramFlowsThrough(kind, innercc, sc, ap, apa, config)
  )
 }

@@ -3631,9 +3663,9 @@ private predicate pathThroughCallable0(
 */
 pragma[noinline]
 private predicate pathThroughCallable(PathNodeMid mid, NodeEx out, CallContext cc, AccessPath ap) {
-  exists(DataFlowCall call, ReturnKindExt kind, AccessPathApprox apa |
-    pathThroughCallable0(call, mid, kind, cc, ap, apa) and
-    out = getAnOutNodeFlow(kind, call, apa, unbindConf(mid.getConfiguration()))
+  exists(DataFlowCall call, ReturnKindExt kind, AccessPathApprox apa, Configuration config |
+    pathThroughCallable0(call, mid, kind, cc, ap, apa, config) and
+    out = getAnOutNodeFlow(kind, call, apa, config)
  )
 }

@@ -3647,10 +3679,11 @@ private module Subpaths {
    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
    NodeEx out, AccessPath apout
  ) {
-    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
-    pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
-      unbindConf(arg.getConfiguration()))
+    exists(Configuration config |
+      pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
+      pathIntoCallable(arg, par, _, innercc, sc, _, config) and
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
+    )
  }

  /**
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll
@@ -110,12 +110,12 @@ abstract class Configuration extends string {
  /**
   * Holds if data may flow from some source to `sink` for this configuration.
   */
-  predicate hasFlowTo(Node sink) { hasFlow(_, sink) }
+  predicate hasFlowTo(Node sink) { this.hasFlow(_, sink) }

  /**
   * Holds if data may flow from some source to `sink` for this configuration.
   */
-  predicate hasFlowToExpr(DataFlowExpr sink) { hasFlowTo(exprNode(sink)) }
+  predicate hasFlowToExpr(DataFlowExpr sink) { this.hasFlowTo(exprNode(sink)) }

  /**
   * Gets the exploration limit for `hasPartialFlow` and `hasPartialFlowRev`
@@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx {
  }

  int getPosition() { this.isParameterOf(_, result) }
+
+  predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) }
 }

 private class RetNodeEx extends NodeEx {
@@ -744,8 +746,12 @@ private module Stage1 {
      returnFlowCallableNodeCand(c, kind, config) and
      p.getEnclosingCallable() = c and
      exists(ap) and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition()
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition()
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -1394,8 +1400,12 @@ private module Stage2 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2083,8 +2093,12 @@ private module Stage3 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2843,8 +2857,12 @@ private module Stage4 {
      fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and
      kind = ret.getKind() and
      p.getPosition() = pos and
-      // we don't expect a parameter to return stored in itself
-      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      // we don't expect a parameter to return stored in itself, unless explicitly allowed
+      (
+        not kind.(ParamUpdateReturnKind).getPosition() = pos
+        or
+        p.allowParameterReturnInSelf()
+      )
    )
  }

@@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome {

  int getParameterPos() { p.isParameterOf(_, result) }

+  ParamNodeEx getParamNode() { result = p }
+
  override string toString() { result = p + ": " + ap }

  predicate hasLocationInfo(
@@ -3170,7 +3190,7 @@ private class AccessPathCons extends AccessPath, TAccessPathCons {
  }

  override string toString() {
-    result = "[" + this.toStringImpl(true) + length().toString() + ")]"
+    result = "[" + this.toStringImpl(true) + this.length().toString() + ")]"
    or
    result = "[" + this.toStringImpl(false)
  }
@@ -3309,9 +3329,11 @@ abstract private class PathNodeImpl extends PathNode {
    result = " <" + this.(PathNodeMid).getCallContext().toString() + ">"
  }

-  override string toString() { result = this.getNodeEx().toString() + ppAp() }
+  override string toString() { result = this.getNodeEx().toString() + this.ppAp() }

-  override string toStringWithContext() { result = this.getNodeEx().toString() + ppAp() + ppCtx() }
+  override string toStringWithContext() {
+    result = this.getNodeEx().toString() + this.ppAp() + this.ppCtx()
+  }

  override predicate hasLocationInfo(
    string filepath, int startline, int startcolumn, int endline, int endcolumn
@@ -3379,11 +3401,11 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {

  override PathNodeImpl getASuccessorImpl() {
    // an intermediate step to another intermediate node
-    result = getSuccMid()
+    result = this.getSuccMid()
    or
    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
    exists(PathNodeMid mid, PathNodeSink sink |
-      mid = getSuccMid() and
+      mid = this.getSuccMid() and
      mid.getNodeEx() = sink.getNodeEx() and
      mid.getAp() instanceof AccessPathNil and
      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
@@ -3460,7 +3482,7 @@ private predicate pathStep(
  exists(TypedContent tc | pathReadStep(mid, node, ap.push(tc), tc, cc)) and
  sc = mid.getSummaryCtx()
  or
-  pathIntoCallable(mid, node, _, cc, sc, _) and ap = mid.getAp()
+  pathIntoCallable(mid, node, _, cc, sc, _, _) and ap = mid.getAp()
  or
  pathOutOfCallable(mid, node, cc) and ap = mid.getAp() and sc instanceof SummaryCtxNone
  or
@@ -3537,14 +3559,16 @@ private predicate pathOutOfCallable(PathNodeMid mid, NodeEx out, CallContext cc)
 */
 pragma[noinline]
 private predicate pathIntoArg(
-  PathNodeMid mid, int i, CallContext cc, DataFlowCall call, AccessPath ap, AccessPathApprox apa
+  PathNodeMid mid, int i, CallContext cc, DataFlowCall call, AccessPath ap, AccessPathApprox apa,
+  Configuration config
 ) {
  exists(ArgNode arg |
    arg = mid.getNodeEx().asNode() and
    cc = mid.getCallContext() and
    arg.argumentOf(call, i) and
    ap = mid.getAp() and
-    apa = ap.getApprox()
+    apa = ap.getApprox() and
+    config = mid.getConfiguration()
  )
 }

@@ -3561,12 +3585,14 @@ private predicate parameterCand(
 pragma[nomagic]
 private predicate pathIntoCallable0(
  PathNodeMid mid, DataFlowCallable callable, int i, CallContext outercc, DataFlowCall call,
-  AccessPath ap
+  AccessPath ap, Configuration config
 ) {
  exists(AccessPathApprox apa |
-    pathIntoArg(mid, i, outercc, call, ap, apa) and
+    pathIntoArg(mid, pragma[only_bind_into](i), outercc, call, ap, pragma[only_bind_into](apa),
+      pragma[only_bind_into](config)) and
    callable = resolveCall(call, outercc) and
-    parameterCand(callable, any(int j | j <= i and j >= i), apa, mid.getConfiguration())
+    parameterCand(callable, pragma[only_bind_into](i), pragma[only_bind_into](apa),
+      pragma[only_bind_into](config))
  )
 }

@@ -3575,12 +3601,13 @@ private predicate pathIntoCallable0(
 * before and after entering the callable are `outercc` and `innercc`,
 * respectively.
 */
+pragma[nomagic]
 private predicate pathIntoCallable(
  PathNodeMid mid, ParamNodeEx p, CallContext outercc, CallContextCall innercc, SummaryCtx sc,
-  DataFlowCall call
+  DataFlowCall call, Configuration config
 ) {
  exists(int i, DataFlowCallable callable, AccessPath ap |
-    pathIntoCallable0(mid, callable, i, outercc, call, ap) and
+    pathIntoCallable0(mid, callable, i, outercc, call, ap, config) and
    p.isParameterOf(callable, i) and
    (
      sc = TSummaryCtxSome(p, ap)
@@ -3610,18 +3637,23 @@ private predicate paramFlowsThrough(
    ap = mid.getAp() and
    apa = ap.getApprox() and
    pos = sc.getParameterPos() and
-    not kind.(ParamUpdateReturnKind).getPosition() = pos
+    // we don't expect a parameter to return stored in itself, unless explicitly allowed
+    (
+      not kind.(ParamUpdateReturnKind).getPosition() = pos
+      or
+      sc.getParamNode().allowParameterReturnInSelf()
+    )
  )
 }

 pragma[nomagic]
 private predicate pathThroughCallable0(
  DataFlowCall call, PathNodeMid mid, ReturnKindExt kind, CallContext cc, AccessPath ap,
-  AccessPathApprox apa
+  AccessPathApprox apa, Configuration config
 ) {
  exists(CallContext innercc, SummaryCtx sc |
-    pathIntoCallable(mid, _, cc, innercc, sc, call) and
-    paramFlowsThrough(kind, innercc, sc, ap, apa, unbindConf(mid.getConfiguration()))
+    pathIntoCallable(mid, _, cc, innercc, sc, call, config) and
+    paramFlowsThrough(kind, innercc, sc, ap, apa, config)
  )
 }

@@ -3631,9 +3663,9 @@ private predicate pathThroughCallable0(
 */
 pragma[noinline]
 private predicate pathThroughCallable(PathNodeMid mid, NodeEx out, CallContext cc, AccessPath ap) {
-  exists(DataFlowCall call, ReturnKindExt kind, AccessPathApprox apa |
-    pathThroughCallable0(call, mid, kind, cc, ap, apa) and
-    out = getAnOutNodeFlow(kind, call, apa, unbindConf(mid.getConfiguration()))
+  exists(DataFlowCall call, ReturnKindExt kind, AccessPathApprox apa, Configuration config |
+    pathThroughCallable0(call, mid, kind, cc, ap, apa, config) and
+    out = getAnOutNodeFlow(kind, call, apa, config)
  )
 }

@@ -3647,10 +3679,11 @@ private module Subpaths {
    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
    NodeEx out, AccessPath apout
  ) {
-    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
-    pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
-      unbindConf(arg.getConfiguration()))
+    exists(Configuration config |
+      pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
+      pathIntoCallable(arg, par, _, innercc, sc, _, config) and
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
+    )
  }

  /**
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll
@@ -801,6 +801,9 @@ private module Cached {
    exists(Node n | getNodeEnclosingCallable(n) = callable | isUnreachableInCallCached(n, call))
  }

+  cached
+  predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) }
+
  cached
  newtype TCallContext =
    TAnyCallContext() or
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll
@@ -175,6 +175,7 @@ module Consistency {

  query predicate postWithInFlow(Node n, string msg) {
    isPostUpdateNode(n) and
+    not clearsContent(n, _) and
    simpleLocalFlowStep(_, n) and
    msg = "PostUpdateNode should not be the target of local flow."
  }
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -803,3 +803,12 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {

 /** Extra data-flow steps needed for lambda flow analysis. */
 predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() }
+
+/**
+ * Holds if flow is allowed to pass from parameter `p` and back to itself as a
+ * side-effect, resulting in a summary from `p` to itself.
+ *
+ * One example would be to allow flow like `p.foo = p.bar;`, which is disallowed
+ * by default as a heuristic.
+ */
+predicate allowParameterReturnInSelf(ParameterNode p) { none() }
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -261,7 +261,10 @@ module Private {

  private newtype TSummaryNodeState =
    TSummaryNodeInputState(SummaryComponentStack s) { inputState(_, s) } or
-    TSummaryNodeOutputState(SummaryComponentStack s) { outputState(_, s) }
+    TSummaryNodeOutputState(SummaryComponentStack s) { outputState(_, s) } or
+    TSummaryNodeClearsContentState(int i, boolean post) {
+      any(SummarizedCallable sc).clearsContent(i, _) and post in [false, true]
+    }

  /**
   * A state used to break up (complex) flow summaries into atomic flow steps.
@@ -308,6 +311,12 @@ module Private {
        this = TSummaryNodeOutputState(s) and
        result = "to write: " + s
      )
+      or
+      exists(int i, boolean post, string postStr |
+        this = TSummaryNodeClearsContentState(i, post) and
+        (if post = true then postStr = " (post)" else postStr = "") and
+        result = "clear: " + i + postStr
+      )
    }
  }

@@ -329,6 +338,11 @@ module Private {
    not parameterReadState(c, state, _)
    or
    state.isOutputState(c, _)
+    or
+    exists(int i |
+      c.clearsContent(i, _) and
+      state = TSummaryNodeClearsContentState(i, _)
+    )
  }

  pragma[noinline]
@@ -364,6 +378,8 @@ module Private {
    parameterReadState(c, _, i)
    or
    isParameterPostUpdate(_, c, i)
+    or
+    c.clearsContent(i, _)
  }

  private predicate callbackOutput(
@@ -436,6 +452,12 @@ module Private {
        )
      )
    )
+    or
+    exists(SummarizedCallable c, int i, ParamNode p |
+      n = summaryNode(c, TSummaryNodeClearsContentState(i, false)) and
+      p.isParameterOf(c, i) and
+      result = getNodeType(p)
+    )
  }

  /** Holds if summary node `out` contains output of kind `rk` from call `c`. */
@@ -461,6 +483,9 @@ module Private {
    exists(SummarizedCallable c, int i |
      isParameterPostUpdate(post, c, i) and
      pre.(ParamNode).isParameterOf(c, i)
+      or
+      pre = summaryNode(c, TSummaryNodeClearsContentState(i, false)) and
+      post = summaryNode(c, TSummaryNodeClearsContentState(i, true))
    )
    or
    exists(SummarizedCallable callable, SummaryComponentStack s |
@@ -478,6 +503,17 @@ module Private {
    )
  }

+  /**
+   * Holds if flow is allowed to pass from parameter `p`, to a return
+   * node, and back out to `p`.
+   */
+  predicate summaryAllowParameterReturnInSelf(ParamNode p) {
+    exists(SummarizedCallable c, int i |
+      c.clearsContent(i, _) and
+      p.isParameterOf(c, i)
+    )
+  }
+
  /** Provides a compilation of flow summaries to atomic data-flow steps. */
  module Steps {
    /**
@@ -504,11 +540,21 @@ module Private {
      // for `StringBuilder.append(x)` with a specified value flow from qualifier to
      // return value and taint flow from argument 0 to the qualifier, then this
      // allows us to infer taint flow from argument 0 to the return value.
-      succ instanceof ParamNode and summaryPostUpdateNode(pred, succ) and preservesValue = true
+      succ instanceof ParamNode and
+      summaryPostUpdateNode(pred, succ) and
+      preservesValue = true
      or
      // Similarly we would like to chain together summaries where values get passed
      // into callbacks along the way.
-      pred instanceof ArgNode and summaryPostUpdateNode(succ, pred) and preservesValue = true
+      pred instanceof ArgNode and
+      summaryPostUpdateNode(succ, pred) and
+      preservesValue = true
+      or
+      exists(SummarizedCallable c, int i |
+        pred.(ParamNode).isParameterOf(c, i) and
+        succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and
+        preservesValue = true
+      )
    }

    /**
@@ -536,10 +582,39 @@ module Private {
    }

    /**
-     * Holds if values stored inside content `c` are cleared when passed as
-     * input of type `input` in `call`.
+     * Holds if values stored inside content `c` are cleared at `n`. `n` is a
+     * synthesized summary node, so in order for values to be cleared at calls
+     * to the relevant method, it is important that flow does not pass over
+     * the argument, either via use-use flow or def-use flow.
+     *
+     * Example:
+     *
+     * ```
+     * a.b = taint;
+     * a.clearB(); // assume we have a flow summary for `clearB` that clears `b` on the qualifier
+     * sink(a.b);
+     * ```
+     *
+     * In the above, flow should not pass from `a` on the first line (or the second
+     * line) to `a` on the third line. Instead, there will be synthesized flow from
+     * `a` on line 2 to the post-update node for `a` on that line (via an intermediate
+     * node where field `b` is cleared).
     */
-    predicate summaryClearsContent(ArgNode arg, Content c) {
+    predicate summaryClearsContent(Node n, Content c) {
+      exists(SummarizedCallable sc, int i |
+        n = summaryNode(sc, TSummaryNodeClearsContentState(i, true)) and
+        sc.clearsContent(i, c)
+      )
+    }
+
+    /**
+     * Holds if values stored inside content `c` are cleared inside a
+     * callable to which `arg` is an argument.
+     *
+     * In such cases, it is important to prevent use-use flow out of
+     * `arg` (see comment for `summaryClearsContent`).
+     */
+    predicate summaryClearsContentArg(ArgNode arg, Content c) {
      exists(DataFlowCall call, int i |
        viableCallable(call).(SummarizedCallable).clearsContent(i, c) and
        arg.argumentOf(call, i)
@@ -599,25 +674,6 @@ module Private {
        ret.getKind() = rk
      )
    }
-
-    /**
-     * Holds if data is written into content `c` of argument `arg` using a flow summary.
-     *
-     * Depending on the type of `c`, this predicate may be relevant to include in the
-     * definition of `clearsContent()`.
-     */
-    predicate summaryStoresIntoArg(Content c, Node arg) {
-      exists(ParamUpdateReturnKind rk, ReturnNodeExt ret, PostUpdateNode out |
-        exists(DataFlowCall call, SummarizedCallable callable |
-          getNodeEnclosingCallable(ret) = callable and
-          viableCallable(call) = callable and
-          summaryStoreStep(_, c, ret) and
-          ret.getKind() = pragma[only_bind_into](rk) and
-          out = rk.getAnOutNode(call) and
-          arg = out.getPreUpdateNode()
-        )
-      )
-    }
  }

  /**