Rust: TaintedPath query

Author: aibaars

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

@@ -0,0 +1,40 @@
+/** Provides classes and predicates to reason about path injection vulnerabilities. */
+
+import rust
+private import codeql.rust.controlflow.BasicBlocks
+private import codeql.rust.controlflow.ControlFlowGraph
+private import codeql.rust.dataflow.DataFlow
+private import codeql.rust.dataflow.TaintTracking
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.internal.DataFlowImpl
+
+/**
+ * Provides default sources, sinks and barriers for detecting path injection
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module TaintedPath {
+  /**
+   * A data flow source for path injection vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for path injection vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A barrier for path injection vulnerabilities.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * An active threat-model source, considered as a flow source.
+   */
+  private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
+
+  /** A sink for path-injection from model data. */
+  private class ModelsAsDataSinks extends Sink {
+    ModelsAsDataSinks() { sinkNode(this, "path-injection") }
+  }
+}

rust/ql/src/queries/security/CWE-022/TaintedPath.qhelp

@@ -0,0 +1,67 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Accessing paths controlled by users can allow an attacker to access unexpected resources. This
+can result in sensitive information being revealed or deleted, or an attacker being able to influence
+behavior by modifying unexpected files.</p>
+
+<p>Paths that are naively constructed from data controlled by a user may be absolute paths, or may contain
+unexpected special characters such as "..". Such a path could point anywhere on the file system.</p>
+
+</overview>
+<recommendation>
+
+<p>Validate user input before using it to construct a file path.</p>
+
+<p>Common validation methods include checking that the normalized path is relative and does not contain
+any ".." components, or checking that the path is contained within a safe folder. The method you should use depends 
+on how the path is used in the application, and whether the path should be a single path component.
+</p>
+
+<p>If the path should be a single path component (such as a file name), you can check for the existence 
+of any path separators ("/" or "\"), or ".." sequences in the input, and reject the input if any are found.
+</p>
+
+<p>
+Note that removing "../" sequences is <i>not</i> sufficient, since the input could still contain a path separator
+followed by "..". For example, the input ".../...//" would still result in the string "../" if only "../" sequences
+are removed.
+</p>
+
+<p>Finally, the simplest (but most restrictive) option is to use an allow list of safe patterns and make sure that
+the user input matches one of these patterns.</p>
+
+</recommendation>
+<example>
+
+<p>In this example, a user-provided file name is read from a HTTP request and then used to access a file
+and send it back to the user. However, a malicious user could enter a file name anywhere on the file system,
+such as "/etc/passwd" or "../../../etc/passwd".</p>
+
+<sample src="examples/TaintedPath.rs" />
+
+<p>
+If the input should only be a file name, you can check that it doesn't contain any path separators or ".." sequences.
+</p>
+
+<sample src="examples/TaintedPathGoodNormalize.rs" />
+
+<p>
+If the input should be within a specific directory, you can check that the resolved path 
+is still contained within that directory.
+</p>
+
+<sample src="examples/TaintedPathGoodFolder.rs" />
+
+</example>
+<references>
+
+<li>
+OWASP:
+<a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.
+</li>
+
+</references>
+</qhelp>

rust/ql/src/queries/security/CWE-022/TaintedPath.ql

@@ -0,0 +1,39 @@
+/**
+ * @name Uncontrolled data used in path expression
+ * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id rust/path-injection
+ * @tags security
+ *       external/cwe/cwe-022
+ *       external/cwe/cwe-023
+ *       external/cwe/cwe-036
+ *       external/cwe/cwe-073
+ *       external/cwe/cwe-099
+ */
+
+import rust
+import codeql.rust.dataflow.DataFlow
+import codeql.rust.dataflow.TaintTracking
+import codeql.rust.security.TaintedPathExtensions
+import TaintedPathFlow::PathGraph
+
+/**
+ * A taint configuration for tainted data that reaches a file access sink.
+ */
+module TaintedPathConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof TaintedPath::Source }
+
+  predicate isSink(DataFlow::Node node) { node instanceof TaintedPath::Sink }
+
+  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof TaintedPath::Barrier }
+}
+
+module TaintedPathFlow = TaintTracking::Global<TaintedPathConfig>;
+
+from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
+where TaintedPathFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
+  "user-provided value"

rust/ql/src/queries/security/CWE-022/examples/TaintedPath.rs

@@ -0,0 +1,9 @@
+use poem::{error::InternalServerError, handler, web::Query, Result};
+use std::{fs, path::PathBuf};
+
+#[handler]
+fn tainted_path_handler(Query(file_name): Query<String>) -> Result<String> {
+    let file_path = PathBuf::from(file_name);
+    // BAD: This could read any file on the filesystem.
+    fs::read_to_string(file_path).map_err(InternalServerError)
+}

rust/ql/src/queries/security/CWE-022/examples/TaintedPathGoodFolder.rs

@@ -0,0 +1,14 @@
+use poem::{error::InternalServerError, handler, http::StatusCode, web::Query, Error, Result};
+use std::{env::home_dir, fs, path::PathBuf};
+
+#[handler]
+fn tainted_path_handler(Query(file_path): Query<String>) -> Result<String, Error> {
+    let public_path = home_dir().unwrap().join("public");
+    let file_path = public_path.join(PathBuf::from(file_path));
+    let file_path = file_path.canonicalize().unwrap();
+    // GOOD: ensure that the path stays within the public folder
+    if !file_path.starts_with(public_path) {
+        return Err(Error::from_status(StatusCode::BAD_REQUEST));
+    }
+    fs::read_to_string(file_path).map_err(InternalServerError)
+}

rust/ql/src/queries/security/CWE-022/examples/TaintedPathGoodNormalize.rs

@@ -0,0 +1,12 @@
+use poem::{error::InternalServerError, handler, http::StatusCode, web::Query, Error, Result};
+use std::{fs, path::PathBuf};
+
+#[handler]
+fn tainted_path_handler(Query(file_name): Query<String>) -> Result<String> {
+    // GOOD: ensure that the filename has no path separators or parent directory references
+    if file_name.contains("..") || file_name.contains("/") || file_name.contains("\\") {
+        return Err(Error::from_status(StatusCode::BAD_REQUEST));
+    }
+    let file_path = PathBuf::from(file_name);
+    fs::read_to_string(file_path).map_err(InternalServerError)
+}