Merge pull request #19080 from geoffw0/deallocation

Rust: Query for dereferencing an invalid pointer

rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.qhelp

@@ -0,0 +1,49 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+
+<p>
+Dereferencing an invalid or dangling pointer may cause undefined behavior. Memory may be corrupted
+causing the program to crash or behave incorrectly, in some cases exposing the program to
+potential attacks.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+When dereferencing a pointer in <code>unsafe</code> code, take care that the pointer is valid and
+points to the intended data. Code may need to be rearranged or additional checks added to ensure
+safety in all circumstances. If possible, rewrite the code using safe Rust types to avoid this
+kind of problem altogether.
+</p>
+
+</recommendation>
+<example>
+
+<p>
+In the following example, <code>std::ptr::drop_in_place</code> is used to execute the destructor
+of an object. However, a pointer to that object is dereferenced later in the program, causing
+undefined behavior:
+</p>
+
+<sample src="AccessInvalidPointerBad.rs" />
+
+<p>
+In this case, undefined behavior can be avoided by rearranging the code so that the dereferencing
+comes before the call to <code>std::ptr::drop_in_place</code>:
+</p>
+
+<sample src="AccessInvalidPointerGood.rs" />
+
+</example>
+<references>
+
+<li>Rust Documentation: <a href="https://doc.rust-lang.org/reference/behavior-considered-undefined.html#dangling-pointers">Behavior considered undefined &gt;&gt; Dangling pointers</a>.</li>
+<li>Rust Documentation: <a href="https://doc.rust-lang.org/std/ptr/index.html#safety">Module ptr - Safety</a>.</li>
+<li>Massachusetts Institute of Technology: <a href="https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/book/second-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer">Unsafe Rust - Dereferencing a Raw Pointer</a>.</li>
+
+</references>
+</qhelp>

rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql

@@ -0,0 +1,42 @@
+/**
+ * @name Access of invalid pointer
+ * @description Dereferencing an invalid or dangling pointer causes undefined behavior and may result in memory corruption.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @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.dataflow.TaintTracking
+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 }
+
+  predicate isBarrierOut(DataFlow::Node node) {
+    // make sinks barriers so that we only report the closest instance
+    isSink(node)
+  }
+}
+
+module AccessInvalidPointerFlow = TaintTracking::Global<AccessInvalidPointerConfig>;
+
+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"

rust/ql/src/queries/security/CWE-825/AccessInvalidPointerBad.rs

@@ -0,0 +1,10 @@
+
+unsafe {
+    std::ptr::drop_in_place(ptr); // executes the destructor of `*ptr`
+}
+
+// ...
+
+unsafe {
+    do_something(&*ptr); // BAD: dereferences `ptr`
+}

rust/ql/src/queries/security/CWE-825/AccessInvalidPointerGood.rs

@@ -0,0 +1,10 @@
+
+unsafe {
+    do_something(&*ptr); // GOOD: dereferences `ptr` while it is still valid
+}
+
+// ...
+
+{
+    std::ptr::drop_in_place(ptr); // executes the destructor of `*ptr`
+}

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

@@ -15,6 +15,7 @@ private import codeql.rust.Diagnostics
 private import codeql.rust.security.SensitiveData
 private import TaintReach
 // 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