github / github/codeql

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

Abierto
#19,983 3 comentarios 0 reacciones 1 asignado Reclamado por @hvitved Ver en GitHub
question
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

(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 https://github.com/github/codeql/issues/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:
```ql
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:
```ql
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"
```ql
/**
* @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;

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](https://github.com/intrigus-lgtm/file-dump/releases/tag/cosmwasm-db-cf413c5ad6a58a87e0be894584f01506f3b2e0af.zip)

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.