github / github/codeql

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

Đang mở
#19,983 3 bình luận 0 reaction 1 người được giao Được @hvitved nhận Xem trên GitHub
question
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

(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)

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.