From dcd016f5be41301d9e55bdc8409ba57a9412cbff Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 13 Mar 2025 17:04:48 +0000 Subject: [PATCH] Rust: Initial version of the query. --- .../frameworks/stdlib/lang-core.model.yml | 8 +++ .../AccessInvalidPointerExtensions.qll | 58 +++++++++++++++++++ .../security/CWE-825/AccessInvalidPointer.ql | 35 +++++++++++ rust/ql/src/queries/summary/Stats.qll | 1 + .../CWE-825/AccessInvalidPointer.expected | 32 ++++++++++ .../CWE-825/AccessInvalidPointer.qlref | 4 ++ .../security/CWE-825/deallocation.rs | 12 ++-- 7 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll create mode 100644 rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql create mode 100644 rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected create mode 100644 rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.qlref diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml b/rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml index fb11ff1e122..db1901bf852 100644 --- a/rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml +++ b/rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml @@ -26,3 +26,11 @@ extensions: - ["lang:core", "crate::ptr::write_volatile", "Argument[1]", "Argument[0].Reference", "value", "manual"] # Str - ["lang:core", "::parse", "Argument[self]", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"] + - addsTo: + pack: codeql/rust-all + extensible: sourceModel + data: + # Alloc + - ["lang:core", "crate::ptr::dangling", "ReturnValue", "pointer-invalidate", "manual"] + - ["lang:core", "crate::ptr::dangling_mut", "ReturnValue", "pointer-invalidate", "manual"] + - ["lang:core", "crate::ptr::null", "ReturnValue", "pointer-invalidate", "manual"] diff --git a/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll b/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll new file mode 100644 index 00000000000..faf9de48def --- /dev/null +++ b/rust/ql/lib/codeql/rust/security/AccessInvalidPointerExtensions.qll @@ -0,0 +1,58 @@ +/** + * Provides classes and predicates for reasoning about accesses to invalid + * pointers. + */ + +import rust +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.dataflow.FlowSource +private import codeql.rust.dataflow.FlowSink +private import codeql.rust.Concepts + +/** + * Provides default sources, sinks and barriers for detecting accesses to + * invalid pointers, as well as extension points for adding your own. + */ +module AccessInvalidPointer { + /** + * A data flow source for invalid pointer accesses, that is, an operation + * where a pointer becomes invalid. + */ + abstract class Source extends DataFlow::Node { } + + /** + * A data flow sink for invalid pointer accesses, that is, a pointer + * dereference. + */ + abstract class Sink extends QuerySink::Range { + override string getSinkType() { result = "AccessInvalidPointer" } + } + + /** + * A barrier for invalid pointer accesses. + */ + abstract class Barrier extends DataFlow::Node { } + + /** + * A pointer invalidation from model data. + */ + private class ModelsAsDataSource extends Source { + ModelsAsDataSource() { sourceNode(this, "pointer-invalidate") } + } + + /** + * A pointer access using the unary `*` operator. + */ + private class DereferenceSink extends Sink { + DereferenceSink() { + exists(PrefixExpr p | p.getOperatorName() = "*" and p.getExpr() = this.asExpr().getExpr()) + } + } + + /** + * A pointer access from model data. + */ + private class ModelsAsDataSink extends Sink { + ModelsAsDataSink() { sinkNode(this, "pointer-access") } + } +} diff --git a/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql b/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql new file mode 100644 index 00000000000..ba124c99d40 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql @@ -0,0 +1,35 @@ +/** + * @name Access of invalid pointer + * @description Dereferencing an invalid or dangling pointer is undefined behavior and may cause memory corruption. + * @kind path-problem + * @problem.severity error + * @security-severity TODO + * @precision TODO + * @id rust/access-invalid-pointer + * @tags reliability + * security + * external/cwe/cwe-476 + * external/cwe/cwe-825 + */ + +import rust +import codeql.rust.dataflow.DataFlow +import codeql.rust.security.AccessInvalidPointerExtensions +import AccessInvalidPointerFlow::PathGraph + +/** + * A data flow configuration for accesses to invalid pointers. + */ +module AccessInvalidPointerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node instanceof AccessInvalidPointer::Source } + + predicate isSink(DataFlow::Node node) { node instanceof AccessInvalidPointer::Sink } + + predicate isBarrier(DataFlow::Node barrier) { barrier instanceof AccessInvalidPointer::Barrier } +} + +module AccessInvalidPointerFlow = DataFlow::Global; + +from AccessInvalidPointerFlow::PathNode sourceNode, AccessInvalidPointerFlow::PathNode sinkNode +where AccessInvalidPointerFlow::flowPath(sourceNode, sinkNode) +select sinkNode.getNode(), sourceNode, sinkNode, "This operation dereferences a pointer that may be $@.", sourceNode.getNode(), "invalid" diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index a2220398b41..f011dda2852 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -12,6 +12,7 @@ private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency private import codeql.rust.Concepts // import all query extensions files, so that all extensions of `QuerySink` are found +private import codeql.rust.security.AccessInvalidPointerExtensions private import codeql.rust.security.CleartextLoggingExtensions private import codeql.rust.security.SqlInjectionExtensions private import codeql.rust.security.WeakSensitiveDataHashingExtensions diff --git a/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected new file mode 100644 index 00000000000..88c1e0d685c --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected @@ -0,0 +1,32 @@ +#select +| deallocation.rs:96:14:96:15 | p1 | deallocation.rs:89:23:89:40 | ...::dangling | deallocation.rs:96:14:96:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:89:23:89:40 | ...::dangling | invalid | +| deallocation.rs:97:14:97:15 | p2 | deallocation.rs:90:21:90:42 | ...::dangling_mut | deallocation.rs:97:14:97:15 | p2 | This operation dereferences a pointer that may be $@. | deallocation.rs:90:21:90:42 | ...::dangling_mut | invalid | +| deallocation.rs:98:14:98:15 | p3 | deallocation.rs:91:23:91:36 | ...::null | deallocation.rs:98:14:98:15 | p3 | This operation dereferences a pointer that may be $@. | deallocation.rs:91:23:91:36 | ...::null | invalid | +edges +| deallocation.rs:89:6:89:7 | p1 | deallocation.rs:96:14:96:15 | p1 | provenance | | +| deallocation.rs:89:23:89:40 | ...::dangling | deallocation.rs:89:23:89:42 | ...::dangling(...) | provenance | Src:MaD:1 MaD:1 | +| deallocation.rs:89:23:89:42 | ...::dangling(...) | deallocation.rs:89:6:89:7 | p1 | provenance | | +| deallocation.rs:90:6:90:7 | p2 | deallocation.rs:97:14:97:15 | p2 | provenance | | +| deallocation.rs:90:21:90:42 | ...::dangling_mut | deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | provenance | Src:MaD:2 MaD:2 | +| deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | deallocation.rs:90:6:90:7 | p2 | provenance | | +| deallocation.rs:91:6:91:7 | p3 | deallocation.rs:98:14:98:15 | p3 | provenance | | +| deallocation.rs:91:23:91:36 | ...::null | deallocation.rs:91:23:91:38 | ...::null(...) | provenance | Src:MaD:3 MaD:3 | +| deallocation.rs:91:23:91:38 | ...::null(...) | deallocation.rs:91:6:91:7 | p3 | provenance | | +models +| 1 | Source: lang:core; crate::ptr::dangling; pointer-invalidate; ReturnValue | +| 2 | Source: lang:core; crate::ptr::dangling_mut; pointer-invalidate; ReturnValue | +| 3 | Source: lang:core; crate::ptr::null; pointer-invalidate; ReturnValue | +nodes +| deallocation.rs:89:6:89:7 | p1 | semmle.label | p1 | +| deallocation.rs:89:23:89:40 | ...::dangling | semmle.label | ...::dangling | +| deallocation.rs:89:23:89:42 | ...::dangling(...) | semmle.label | ...::dangling(...) | +| deallocation.rs:90:6:90:7 | p2 | semmle.label | p2 | +| deallocation.rs:90:21:90:42 | ...::dangling_mut | semmle.label | ...::dangling_mut | +| deallocation.rs:90:21:90:44 | ...::dangling_mut(...) | semmle.label | ...::dangling_mut(...) | +| deallocation.rs:91:6:91:7 | p3 | semmle.label | p3 | +| deallocation.rs:91:23:91:36 | ...::null | semmle.label | ...::null | +| deallocation.rs:91:23:91:38 | ...::null(...) | semmle.label | ...::null(...) | +| deallocation.rs:96:14:96:15 | p1 | semmle.label | p1 | +| deallocation.rs:97:14:97:15 | p2 | semmle.label | p2 | +| deallocation.rs:98:14:98:15 | p3 | semmle.label | p3 | +subpaths diff --git a/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.qlref b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.qlref new file mode 100644 index 00000000000..c8e593a6a31 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.qlref @@ -0,0 +1,4 @@ +query: queries/security/CWE-825/AccessInvalidPointer.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/rust/ql/test/query-tests/security/CWE-825/deallocation.rs b/rust/ql/test/query-tests/security/CWE-825/deallocation.rs index 1ad8d72c008..c880ed9fd08 100644 --- a/rust/ql/test/query-tests/security/CWE-825/deallocation.rs +++ b/rust/ql/test/query-tests/security/CWE-825/deallocation.rs @@ -86,16 +86,16 @@ pub fn test_libc() { // --- std::ptr --- pub fn test_ptr_invalid(do_dangerous_accesses: bool) { - let p1: *const i64 = std::ptr::dangling(); - let p2: *mut i64 = std::ptr::dangling_mut(); - let p3: *const i64 = std::ptr::null(); + let p1: *const i64 = std::ptr::dangling(); // $ Source=dangling + let p2: *mut i64 = std::ptr::dangling_mut(); // $ Source=dangling_mut + let p3: *const i64 = std::ptr::null(); // $ Source=null if do_dangerous_accesses { unsafe { // (a segmentation fault occurs in the code below) - let v1 = *p1; // $ MISSING: Alert - let v2 = *p2; // $ MISSING: Alert - let v3 = *p3; // $ MISSING: Alert + let v1 = *p1; // $ Alert[rust/access-invalid-pointer]=dangling + let v2 = *p2; // $ Alert[rust/access-invalid-pointer]=dangling_mut + let v3 = *p3; // $ Alert[rust/access-invalid-pointer]=null println!(" v1 = {v1} (!)"); println!(" v2 = {v2} (!)"); println!(" v3 = {v3} (!)");