Skip to content

[Rust] weird behavior in dataflow when trying to select a specific node #19983

Open
@intrigus-lgtm

Description

@intrigus-lgtm

(Apologies for the vague issue title, but I just didn't know any better title...)

Description of the issue

(I don't think this is related to #19982 but linking anyway)

When running the "working base version" (see below for code), I successfully find flow from the argument data to remaining_len - 4 - tail_len..remaining_len - 4 just as I wanted:

Image

source change

The problem appears when I change the source predicate to this more general version (all results of read_region are assumed to return user-controlled data) where I've manually verified that the result of read_region is used in a call with a data argument:

predicate isSource(DataFlow::Node node) {
    exists(CallExpr ce, Function f | f.getName().getText() = "read_region" |
      node.asExpr().getExpr() = ce and ce.getStaticTarget() = f
    ) and
    node.getLocation().getStartLine() = 727
  }

This results in ZERO alerts/#select/nodes/subpaths/edges.

any() filtering in sink

So obviously, I tried to debug it and for that I changed the sink predicate to this:

  predicate isSink(DataFlow::Node node) {
    any()
  }

(Yes, yes, there is partial flow and stuff, but good old any works pretty well in simple cases :P)
This is exactly the flow I'm trying to find.
Image

RangeExpr filtering in sink

What happens if I change my sink predicate to check whether the node is a RangeExpr using node.asExpr().getExpr() instanceof RangeExpr? That should work, because earlier, sink.getNode().asExpr().getExpr().getAPrimaryQlClass() returned RangeExpr, right?

Well, we get ZERO alerts/.../... again.

getStartLine filterting in sink

Okay, then let's change the predicate to node.getLocation().getStartLine() = 20 because that should also work as we verified that the node of interested has its start line indeed in line 20.

This does give us four results:
but our RangeExpr is nowhere to be found!?

Image

two sections.rs files

There exist two sections.rs files:
cosmwasm-db-cf413c5ad6a58a87e0be894584f01506f3b2e0af^/src.zip/home/sg/dev/cosmwasm/packages/std/src/sections.rs
and
cosmwasm-db-cf413c5ad6a58a87e0be894584f01506f3b2e0af^/src.zip/home/sg/dev/cosmwasm/packages/vm/src/sections.rs
but I don't really see how that could be relevant, but highlighting anyway.

Summary

I'm not sure whether I did something dumb or whether I've run into a bug :)

Reproduction steps

git clone https://github.com/CosmWasm/cosmwasm.git
git checkout cf413c5ad6a58a87e0be894584f01506f3b2e0af^
codeql database create --language=rust "cosmwasm-db-cf413c5ad6a58a87e0be894584f01506f3b2e0af^" --overwrite

"Working base version"

/**
 * @name Empty block
 * @kind path-problem
 * @problem.severity warning
 * @id rust/example/empty-block
 */

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

module MyConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node node) {
    node.asPat().getPat().(IdentPat).getName().getText() = "data"
  }

  predicate isSink(DataFlow::Node node) {
    exists(IndexExpr ie | node.asExpr().getExpr().(RangeExpr) = ie.getIndex())
  }

  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
    // If the start/end of a range is tainted, then the whole range is tainted.
    exists(RangeExpr re |
      pred.asExpr().getExpr() = [re.getStart(), re.getEnd()] and
      succ.asExpr().getExpr() = re
    )
    or
    // Propagate taint from `tainted` to the result of `tainted?`
    exists(TryExpr te |
      te.getExpr() = pred.asExpr().getExpr() and
      te = succ.asExpr().getExpr()
    )
    or
    // Propagate taint from `tainted` to `[tainted]`
    exists(ArrayListExpr ale |
      ale.getAnExpr() = pred.asExpr().getExpr() and
      ale = succ.asExpr().getExpr()
    )
    or
    // Propagate taint from `tainted` to `from_be_bytes(tainted)`
    exists(CallExpr ce, Function f |
      pred.asExpr().getExpr() = ce.getArg(0) and
      succ.asExpr().getExpr() = ce and
      ce.getStaticTarget() = f and
      f.getName().getText() = "from_be_bytes"
    )
    or
    // Propagate taint from `tainted` to `tainted as usize`
    exists(CastExpr ce |
      pred.asExpr().getExpr() = ce.getExpr() and
      succ.asExpr().getExpr() = ce and
      pred.getLocation().getFile().getAbsolutePath().matches("%section%")
    )
  }
}

module Flow = TaintTracking::Global<MyConfig>;

import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink, source, sink,
  "PrimaryQlClass: " + sink.getNode().asExpr().getExpr().getAPrimaryQlClass() + "start: " +
    sink.getLocation().getStartLine()

Files

cosmwasm-db-cf413c5ad6a58a87e0be894584f01506f3b2e0af^.zip

Metadata

Metadata

Assignees

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