Skip to content

Rust: slice elements do not flow to for_each closure parameters #23064

Open

Description

@bdrodes

Description of the issue

Data flow does not reach the parameter of a closure passed to for_each on a slice iterator, as in a.iter().for_each(|x| sink(x)). The equivalent for x in a.iter() { sink(x) } loop works. The cause appears to be the model precedence rule: the callback summary added in #22287 hides the manual Iterator::for_each model for this method.

Example

fn source() -> String {
    String::from("secret")
}

fn sink(s: &String) {
    println!("{s}");
}

fn main() {
    let a = [source()];
    a.iter().for_each(|x| sink(x)); // A
    for x in a.iter() {
        sink(x); // B
    }
}

The program prints secret twice: in both A and B, x is the element of a.

Query: taint tracking from each source() call to each sink argument.

import rust
import codeql.rust.dataflow.DataFlow
import codeql.rust.dataflow.TaintTracking

module Config implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node n) {
    exists(Call c | c.getStaticTarget().getName().getText() = "source" | n.asExpr() = c)
  }

  predicate isSink(DataFlow::Node n) {
    exists(Call c | c.getStaticTarget().getName().getText() = "sink" |
      n.asExpr() = c.getPositionalArgument(0)
    )
  }
}

module Flow = TaintTracking::Global<Config>;

from DataFlow::Node source, DataFlow::Node sink
where Flow::flow(source, sink)
select sink, source

Result, the same on CodeQL 2.27.4 (codeql/rust-all 0.2.25) and 2.27.2 (0.2.23):

sink source
x at line 13 (B) source() at line 10

There is no row for A (line 11).

Cause (our reading of the source)

  1. core::slice::iter::Iter has its own for_each (it overrides the trait's default method), so the call resolves to <core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::for_each.
  2. The manual model <core::iter::traits::iterator::Iterator>::for_each: Argument[self].Element -> Argument[0].Parameter[0] (rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml) reaches that method only as an inherited model. interpretPath in ModelsAsData.qll gives inherited models provenance hq-generated and isExact = false.
  3. SummarizedCallableWithCallback in ModelsAsData.qll (Rust: Assume callbacks will be invoked in library functions #22287) gives the same method a heuristic-callback summary, Argument[0] -> Argument[0].Parameter[closure-self], with provenance hq-generated and isExact = true.
  4. RelevantModel::isRelevantModel in the shared FlowSummaryImpl.qll drops an inexact model when the same callable has an exact model with the same verification. So the inherited Element -> Parameter[0] model is dropped. The only summary left says that the closure is called, but not with what.

The comment in interpretPath says inherited models are generated "so that source code definitions and exact generated models take precedence". That fits generated models of the method itself. The callback summary carries no element flow, though, so here it hides the only model that does.

Confirming the cause

Adding a manual model for the exact method restores the flow at A. The first row restores the missing element-to-parameter flow. The second row is not needed for this query; it keeps the closure edge from #22287, which a manual model on the method would otherwise suppress:

extensions:
  - addsTo:
      pack: codeql/rust-all
      extensible: summaryModel
    data:
      - ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::for_each", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
      - ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::for_each", "Argument[0]", "Argument[0].Parameter[closure-self]", "value", "manual"]

With these two rows (tested on CodeQL 2.27.2), the query finds both A (x at line 11) and B (x at line 13), each from source() at line 10.

Possible fix

A suggestion only; we have not tested it. Don't let the exact heuristic-callback summary cause other, inexact summaries of the same callable to be discarded. The callback summary itself can stay subject to the normal manual and neutral-model precedence rules.

Changing the callback summary to isExact = false instead is not enough: an exact generated summary could then suppress it, losing the captured-variable flow added by #22287.

Scope

We reproduced this only for for_each on core::slice::iter::Iter. Other iterator types that override a closure-taking Iterator method may have the same problem; we have not tested them.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions