JS: Disallow consecutive captured contents

javascript/ql/lib/semmle/javascript/dataflow/internal/Contents.qll

@@ -81,6 +81,7 @@ module Private {
     MkPromiseFilter() or
     MkIteratorFilter() or
     MkAnyProperty() or
+    MkAnyCapturedContent() or
     // The following content sets are used exclusively as an intermediate value in flow summaries.
     // These are encoded as a ContentSummaryComponent, although the flow graphs we generate are different
     // than an ordinary content component. These special content sets should never appear in a step.
@@ -239,6 +240,9 @@ module Public {
         or
         result instanceof MkArrayElementUnknown
       )
+      or
+      this = ContentSet::anyCapturedContent() and
+      result instanceof Private::MkCapturedContent
     }
 
     /** Gets the singleton content to be accessed. */
@@ -278,6 +282,9 @@ module Public {
       this = MkAnyPropertyDeep() and result = "AnyMemberDeep"
       or
       this = MkArrayElementDeep() and result = "ArrayElementDeep"
+      or
+      this = MkAnyCapturedContent() and
+      result = "AnyCapturedContent"
     }
   }
 
@@ -477,5 +484,10 @@ module Public {
         else result = property(propertyName)
       )
     }
+
+    /**
+     * Gets a content set that reads from all captured variables stored on a function.
+     */
+    ContentSet anyCapturedContent() { result = Private::MkAnyCapturedContent() }
   }
 }

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

@@ -839,6 +839,13 @@ predicate clearsContent(Node n, ContentSet c) {
   c = MkPromiseFilter()
   or
   any(AdditionalFlowInternal flow).clearsContent(n, c)
+  or
+  // When a function `f` captures itself, all its access paths can be prefixed by an arbitrary number of `f.f.f...`.
+  // When multiple functions `f,g` capture each other, these prefixes can become interleaved, like `f.g.f.g...`.
+  // To avoid creating these trivial prefixes, we never allow two consecutive captured variables in the access path.
+  // We implement this rule by clearing any captured-content before storing into another captured-content.
+  VariableCaptureOutput::storeStep(getClosureNode(n), _, _) and
+  c = MkAnyCapturedContent()
 }
 
 /**

javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll

@@ -55,11 +55,7 @@ module VariableCaptureConfig implements InputSig {
     CapturedVariable() {
       DataFlowImplCommon::forceCachingInSameStage() and
       this.isCaptured() and
-      not isTopLevelLike(this.getDeclaringContainer()) and
-      // Exclude variables that just contain a function
-      // TODO: explain why
-      // TODO: also exclude if only use of variable is to call it. Handles case where variable is just alias for top-level function
-      not exists(getLambdaFromVariable(this))
+      not isTopLevelLike(this.getDeclaringContainer())
     }
 
     Callable getCallable() { result = this.getDeclaringContainer().getFunctionBoundary() }