Rust: ? does not propagate early-returned Err or None in data flow #23065

Description of the issue

When x? takes its early-return path, it returns a value derived from x to the caller of the enclosing function. For Result, that is an Err, made by FromResidual::from_residual; the standard implementation applies From::from to the error. For Option, it is None. CodeQL does not model this return, so neither data flow nor taint tracking follows the error to the callers. #22903 notes the gap in passing ("The early return itself, the Err or None held by x, was not modelled as returned data before and still is not"). We found no issue tracking it, so this one adds a standalone reproduction.

Example

#[derive(Debug)]
struct MyError(String);

fn sink<T: std::fmt::Debug>(t: T) {
    println!("{t:?}");
}

fn always_fails() -> Result<u32, MyError> {
    Err(MyError(String::from("user input")))
}

// `?` returns the `Err` from `always_fails()` to the caller here.
fn early_return() -> Result<u32, MyError> {
    always_fails()?;
    Ok(0)
}

fn nothing() -> Option<u32> {
    None
}

// `?` returns the `None` from `nothing()` to the caller here.
fn early_none() -> Option<u32> {
    nothing()?;
    Some(0)
}

fn main() {
    sink(early_return());
    sink(early_none());
}

The program prints Err(MyError("user input")) and then None.

Query: data flow and taint tracking from every literal, call and path expression 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) {
    n.asExpr() instanceof LiteralExpr or
    n.asExpr() instanceof CallExpr or
    n.asExpr() instanceof PathExpr
  }

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

module ValueFlow = DataFlow::Global<Config>;

module TaintFlow = TaintTracking::Global<Config>;

from DataFlow::Node source, DataFlow::Node sink, string mode
where
  ValueFlow::flow(source, sink) and mode = "value"
  or
  TaintFlow::flow(source, sink) and mode = "taint"
select sink, mode, source

Expected: the error created in always_fails() reaches the first sink, and the None from nothing() reaches the second.

Result on CodeQL 2.27.4 (codeql/rust-all 0.2.25); data flow and taint tracking give the same rows:

On CodeQL 2.27.2 (codeql/rust-all 0.2.23), taint tracking reported the missing sources, but only through the incorrect ? return flow that #22903 fixed. Data flow did not report them.

Cause (our reading of the source)

• isReturnedValue in rust/ql/lib/codeql/rust/dataflow/internal/Node.qll leaves out the early-return edge of a TryExpr, correctly since Rust: Do not treat the early return of ? as returning the unwrapped value #22903: the value of x? itself is the unwrapped Ok/Some payload, not what the early return returns.
• Nothing else models the early return. The only data flow step for TryExpr in DataFlowImpl.qll reads the Ok/Some payload on the normal path.

Why it matters

Errors often carry user input or secrets, and ? is the usual way Rust code passes them up. A query that tracks an error to a log call in a caller (log injection, cleartext logging) misses that path. For example, check(input)? inside a helper, then if let Err(e) = helper() { log::error!("{e:?}") } in the caller.

Possible fix

A suggestion only; we have not tested an implementation. Model the early-return edge of x? as returning the value produced by FromResidual::from_residual:

• for Result: flow from the Err payload of x, through the error conversion, into the Err of the returned value;
• for Option: the returned None.

The database has no call for this implicit FromResidual step, so it may need a synthetic flow step rather than an ordinary call summary.

Scope

We tested Result and Option only. Other types that support ?, such as ControlFlow and Poll, may need the same flow; we have not tested them.