Skip to content

Commit 40f629e

Browse files
committed
Rust: Add barriers for rust/access-invalid-pointer
1 parent 6ddb9c7 commit 40f629e

File tree

3 files changed

+67
-10
lines changed

3 files changed

+67
-10
lines changed

rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
*/
55

66
import rust
7+
private import codeql.rust.Concepts
78
private import codeql.rust.dataflow.DataFlow
9+
private import codeql.rust.dataflow.FlowSink
810
private import codeql.rust.security.AccessInvalidPointerExtensions
911
private import codeql.rust.internal.Type
1012
private import codeql.rust.internal.TypeInference as TypeInference
@@ -29,10 +31,11 @@ module AccessAfterLifetime {
2931

3032
/**
3133
* A data flow sink for accesses to a pointer after its lifetime has ended,
32-
* that is, a dereference. We re-use the same sinks as for the accesses to
33-
* invalid pointers query.
34+
* that is, a dereference.
3435
*/
35-
class Sink = AccessInvalidPointer::Sink;
36+
abstract class Sink extends QuerySink::Range {
37+
override string getSinkType() { result = "AccessAfterLifetime" }
38+
}
3639

3740
/**
3841
* A barrier for accesses to a pointer after its lifetime has ended.
@@ -117,6 +120,18 @@ module AccessAfterLifetime {
117120
override Expr getTarget() { result = targetValue }
118121
}
119122

123+
/**
124+
* A pointer access using the unary `*` operator.
125+
*/
126+
private class DereferenceSink extends Sink {
127+
DereferenceSink() { any(DerefExpr p).getExpr() = this.asExpr() }
128+
}
129+
130+
/** A pointer access from model data. */
131+
private class ModelsAsDataSink extends Sink {
132+
ModelsAsDataSink() { sinkNode(this, "pointer-access") }
133+
}
134+
120135
/**
121136
* A barrier for nodes inside closures, as we don't model lifetimes of
122137
* variables through closures properly.

rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
*/
55

66
import rust
7+
private import codeql.rust.elements.Call
78
private import codeql.rust.dataflow.DataFlow
89
private import codeql.rust.dataflow.FlowSource
910
private import codeql.rust.dataflow.FlowSink
1011
private import codeql.rust.Concepts
1112
private import codeql.rust.dataflow.internal.Node
1213
private import codeql.rust.security.Barriers as Barriers
14+
private import codeql.rust.internal.TypeInference as TypeInference
15+
private import codeql.rust.internal.Type
1316

1417
/**
1518
* Provides default sources, sinks and barriers for detecting accesses to
@@ -47,20 +50,58 @@ module AccessInvalidPointer {
4750
ModelsAsDataSource() { sourceNode(this, "pointer-invalidate") }
4851
}
4952

50-
/**
51-
* A pointer access using the unary `*` operator.
52-
*/
53+
/** A raw pointer access using the unary `*` operator. */
5354
private class DereferenceSink extends Sink {
54-
DereferenceSink() { any(DerefExpr p).getExpr() = this.asExpr() }
55+
DereferenceSink() {
56+
exists(Expr p, DerefExpr d | p = d.getExpr() and p = this.asExpr() |
57+
// Dereferencing a raw pointer is an unsafe operation. Hence relevant
58+
// dereferences must occur inside code marked as unsafe.
59+
// See: https://doc.rust-lang.org/reference/types/pointer.html#r-type.pointer.raw.safety
60+
(p.getEnclosingBlock*().isUnsafe() or p.getEnclosingCallable().(Function).isUnsafe()) and
61+
(not exists(TypeInference::inferType(p)) or TypeInference::inferType(p) instanceof PtrType)
62+
)
63+
}
5564
}
5665

57-
/**
58-
* A pointer access from model data.
59-
*/
66+
/** A pointer access from model data. */
6067
private class ModelsAsDataSink extends Sink {
6168
ModelsAsDataSink() { sinkNode(this, "pointer-access") }
6269
}
6370

71+
private class BarrierCall extends Barrier {
72+
BarrierCall() {
73+
exists(Call call, ArgumentPosition pos, string canonicalName |
74+
call.getStaticTarget().getCanonicalPath() = canonicalName and
75+
this.asExpr() = call.getArgument(pos)
76+
|
77+
canonicalName = "<core::ptr::non_null::NonNull>::new" and pos.asPosition() = 0
78+
)
79+
}
80+
}
81+
82+
private class NumericTypeBarrier extends Barrier instanceof Barriers::NumericTypeBarrier { }
83+
84+
private class BooleanTypeBarrier extends Barrier instanceof Barriers::BooleanTypeBarrier { }
85+
86+
private class FieldlessEnumTypeBarrier extends Barrier instanceof Barriers::FieldlessEnumTypeBarrier
87+
{ }
88+
89+
private class DefaultBarrier extends Barrier {
90+
DefaultBarrier() {
91+
// A barrier for calls that statically resolve to the `Default::default`
92+
// trait function. Such calls are imprecise, and can always resolve to the
93+
// implementations for raw pointers that return a null pointer. This
94+
// creates many false positives in combination with other inaccuracies
95+
// (too many `pointer-access` sinks created by the model generator).
96+
//
97+
// We could try removing this barrier in the future when either 1/ the
98+
// model generator creates fewer spurious sinks or 2/ data flow for calls
99+
// to trait functions is more precise.
100+
this.asExpr().(Call).getStaticTarget().getCanonicalPath() =
101+
"<_ as core::default::Default>::default"
102+
}
103+
}
104+
64105
/**
65106
* A barrier for invalid pointer access vulnerabilities for values checked to
66107
* be non-`null`.

rust/ql/src/queries/summary/Stats.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ private import codeql.rust.security.SensitiveData
1818
private import TaintReach
1919
// import all query extensions files, so that all extensions of `QuerySink` are found
2020
private import codeql.rust.security.regex.RegexInjectionExtensions
21+
private import codeql.rust.security.AccessAfterLifetimeExtensions
2122
private import codeql.rust.security.AccessInvalidPointerExtensions
2223
private import codeql.rust.security.CleartextLoggingExtensions
2324
private import codeql.rust.security.CleartextStorageDatabaseExtensions

0 commit comments

Comments
 (0)