Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: Add some flow source models #18069

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions rust/ql/lib/codeql/rust/Concepts.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

private import codeql.rust.dataflow.DataFlow
private import codeql.threatmodels.ThreatModels
private import codeql.rust.Frameworks

/**
* A data flow source for a specific threat-model.
Expand Down Expand Up @@ -46,6 +47,63 @@ class ActiveThreatModelSource extends ThreatModelSource {
ActiveThreatModelSource() { currentThreatModel(this.getThreatModel()) }
}

/**
* A data flow source corresponding to the program's command line arguments or path.
*/
final class CommandLineArgsSource = CommandLineArgsSource::Range;

/**
* Provides a class for modeling new sources for the program's command line arguments or path.
*/
module CommandLineArgsSource {
/**
* A data flow source corresponding to the program's command line arguments or path.
*/
abstract class Range extends ThreatModelSource::Range {
override string getThreatModel() { result = "commandargs" }

override string getSourceType() { result = "CommandLineArgs" }
}
}

/**
* A data flow source corresponding to the program's environment.
*/
final class EnvironmentSource = EnvironmentSource::Range;

/**
* Provides a class for modeling new sources for the program's environment.
*/
module EnvironmentSource {
/**
* A data flow source corresponding to the program's environment.
*/
abstract class Range extends ThreatModelSource::Range {
override string getThreatModel() { result = "environment" }

override string getSourceType() { result = "EnvironmentSource" }
}
}

/**
* A data flow source for remote (network) data.
*/
final class RemoteSource = RemoteSource::Range;

/**
* Provides a class for modeling new sources of remote (network) data.
*/
module RemoteSource {
/**
* A data flow source for remote (network) data.
*/
abstract class Range extends ThreatModelSource::Range {
override string getThreatModel() { result = "remote" }

override string getSourceType() { result = "RemoteSource" }
}
}

/**
* A data-flow node that constructs a SQL statement.
*
Expand Down
6 changes: 6 additions & 0 deletions rust/ql/lib/codeql/rust/Frameworks.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* This file imports all models of frameworks and libraries.
*/

private import codeql.rust.frameworks.Reqwest
private import codeql.rust.frameworks.stdlib.Env
19 changes: 19 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/Reqwest.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Provides modeling for the `reqwest` library.
*/

private import rust
private import codeql.rust.Concepts

/**
* A call to `reqwest::get` or `reqwest::blocking::get`.
*/
private class ReqwestGet extends RemoteSource::Range {
CallExpr ce;
Fixed Show fixed Hide fixed

ReqwestGet() {
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
this.asExpr().getExpr() = ce and
ce.getExpr().(PathExpr).getPath().getResolvedCrateOrigin().matches("%reqwest") and
ce.getExpr().(PathExpr).getPath().getResolvedPath() = ["crate::get", "crate::blocking::get"]
}
}
36 changes: 36 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/Env.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* Provides modeling for the `std::env` library.
*/

private import rust
private import codeql.rust.Concepts

/**
* A call to `std::env::args` or `std::env::args_os`.
*/
private class StdEnvArgs extends CommandLineArgsSource::Range {
StdEnvArgs() {
this.asExpr().getExpr().(CallExpr).getExpr().(PathExpr).getPath().getResolvedPath() =
["crate::env::args", "crate::env::args_os"]
}
}

/**
* A call to `std::env::current_dir`, `std::env::current_exe` or `std::env::home_dir`.
*/
private class StdEnvDir extends CommandLineArgsSource::Range {
StdEnvDir() {
this.asExpr().getExpr().(CallExpr).getExpr().(PathExpr).getPath().getResolvedPath() =
["crate::env::current_dir", "crate::env::current_exe", "crate::env::home_dir"]
}
}

/**
* A call to `std::env::var`, `std::env::var_os`, `std::env::vars` or `std::env::vars_os`.
*/
private class StdEnvVar extends EnvironmentSource::Range {
StdEnvVar() {
this.asExpr().getExpr().(CallExpr).getExpr().(PathExpr).getPath().getResolvedPath() =
["crate::env::var", "crate::env::var_os", "crate::env::vars", "crate::env::vars_os"]
}
}
5 changes: 5 additions & 0 deletions rust/ql/src/queries/summary/SummaryStats.ql
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import rust
import codeql.rust.Concepts
import codeql.rust.Diagnostics
import Stats

Expand Down Expand Up @@ -37,4 +38,8 @@ where
key = "Inconsistencies - CFG" and value = getTotalCfgInconsistencies()
or
key = "Inconsistencies - data flow" and value = getTotalDataFlowInconsistencies()
or
key = "Taint sources - total" and value = count(ThreatModelSource s)
or
key = "Taint sources - active" and value = count(ActiveThreatModelSource s)
select key, value
18 changes: 18 additions & 0 deletions rust/ql/src/queries/summary/TaintSources.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @name Taint Sources
* @description List all sources of untrusted input that have been idenfitied
* in the database.
* @kind problem
* @problem.severity info
* @id rust/summary/taint-sources
* @tags summary
*/

import rust
import codeql.rust.Concepts

from ThreatModelSource s, string defaultString
where
if s instanceof ActiveThreatModelSource then defaultString = " (DEFAULT)" else defaultString = ""
select s,
"Flow source '" + s.getSourceType() + "' of type " + s.getThreatModel() + defaultString + "."
Empty file.
21 changes: 21 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/InlineFlow.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import rust
import codeql.rust.dataflow.DataFlow
import codeql.rust.Concepts
import utils.InlineFlowTest

/**
* Configuration for flow from any threat model source to an argument of a function called `sink`.
*/
module MyFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource }

predicate isSink(DataFlow::Node sink) {
any(CallExpr call | call.getExpr().(PathExpr).getPath().toString() = "sink")
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
.getArgList()
.getAnArg() = sink.asExpr().getExpr()
}
}

module MyFlowTest = TaintFlowTest<MyFlowConfig>;

import MyFlowTest
17 changes: 17 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/TaintSources.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
| test.rs:8:10:8:30 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:9:10:9:33 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:11:16:11:36 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:12:16:12:39 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:17:25:17:40 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:22:25:22:43 | CallExpr | Flow source 'EnvironmentSource' of type environment. |
| test.rs:29:29:29:44 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:32:16:32:31 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:33:16:33:34 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:40:16:40:31 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:44:16:44:34 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:50:15:50:37 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:51:15:51:37 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:52:16:52:35 | CallExpr | Flow source 'CommandLineArgs' of type commandargs. |
| test.rs:60:26:60:70 | CallExpr | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:63:26:63:70 | CallExpr | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:66:26:66:60 | CallExpr | Flow source 'RemoteSource' of type remote (DEFAULT). |
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: queries/summary/TaintSources.ql
postprocess: utils/InlineExpectationsTestQuery.ql
3 changes: 3 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/options.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
qltest_cargo_check: true
qltest_dependencies:
- reqwest = { version = "0.12.9", features = ["blocking"] }
36 changes: 36 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/reqwest.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@

// --- stubs for the "reqwest" library ---

/*
--- we don't seem to have a way to use this, hence we currently test against the real reqwest library
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redsun82 I tried to stub reqwest, it has a much simpler interface than sqlx. But I ran into a problem that it's not possible (I think) to emulate the correct paths for the library and import it into a test.rs at the moment. When we can, we can switch to a stubbing approach for reqwest at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(fixing this will probably be follow-up work, what we have works well enough right now)

#[derive(Debug)]
pub struct Error { }

pub mod blocking {
pub struct Response { }
impl Response {
pub fn text(self) -> Result<String, super::Error> {
Ok("".to_string())
}
}

pub fn get<T>(url: T) -> Result<Response, super::Error> {
let _url = url;

Ok(Response {})
}
}

pub struct Response { }
impl Response {
pub async fn text(self) -> Result<String, Error> {
Ok("".to_string())
}
}

pub async fn get<T>(url: T) -> Result<Response, Error> {
let _url = url;

Ok(Response {})
}
*/
70 changes: 70 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#![allow(deprecated)]

fn sink<T>(_: T) { }

// --- tests ---

fn test_env_vars() {
sink(std::env::var("HOME")); // $ Alert[rust/summary/taint-sources] hasTaintFlow
sink(std::env::var_os("PATH")); // $ Alert[rust/summary/taint-sources] hasTaintFlow

let var1 = std::env::var("HOME").expect("HOME not set"); // $ Alert[rust/summary/taint-sources]
let var2 = std::env::var_os("PATH").unwrap(); // $ Alert[rust/summary/taint-sources]

sink(var1); // $ MISSING: hasTaintFlow
sink(var2); // $ MISSING: hasTaintFlow
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved

for (key, value) in std::env::vars() { // $ Alert[rust/summary/taint-sources]
sink(key); // $ MISSING: hasTaintFlow
sink(value); // $ MISSING: hasTaintFlow
}

for (key, value) in std::env::vars_os() { // $ Alert[rust/summary/taint-sources]
sink(key); // $ MISSING: hasTaintFlow
sink(value); // $ MISSING: hasTaintFlow
}
}

fn test_env_args() {
let args: Vec<String> = std::env::args().collect(); // $ Alert[rust/summary/taint-sources]
let my_path = &args[0];
let arg1 = &args[1];
let arg2 = std::env::args().nth(2).unwrap(); // $ Alert[rust/summary/taint-sources]
let arg3 = std::env::args_os().nth(3).unwrap(); // $ Alert[rust/summary/taint-sources]

sink(my_path); // $ MISSING: hasTaintFlow
sink(arg1); // $ MISSING: hasTaintFlow
sink(arg2); // $ MISSING: hasTaintFlow
sink(arg3); // $ MISSING: hasTaintFlow

for arg in std::env::args() { // $ Alert[rust/summary/taint-sources]
sink(arg); // $ MISSING: hasTaintFlow
}

for arg in std::env::args_os() { // $ Alert[rust/summary/taint-sources]
sink(arg); // $ MISSING: hasTaintFlow
}
}

fn test_env_dirs() {
let dir = std::env::current_dir().expect("FAILED"); // $ Alert[rust/summary/taint-sources]
let exe = std::env::current_exe().expect("FAILED"); // $ Alert[rust/summary/taint-sources]
let home = std::env::home_dir().expect("FAILED"); // $ Alert[rust/summary/taint-sources]

sink(dir); // $ MISSING: hasTaintFlow
sink(exe); // $ MISSING: hasTaintFlow
sink(home); // $ MISSING: hasTaintFlow
}

async fn test_reqwest() -> Result<(), reqwest::Error> {
let remote_string1 = reqwest::blocking::get("http://example.com/")?.text()?; // $ Alert[rust/summary/taint-sources]
sink(remote_string1); // $ MISSING: hasTaintFlow

let remote_string2 = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap(); // $ Alert[rust/summary/taint-sources]
sink(remote_string2); // $ MISSING: hasTaintFlow

let remote_string3 = reqwest::get("http://example.com/").await?.text().await?; // $ Alert[rust/summary/taint-sources]
sink(remote_string3); // $ MISSING: hasTaintFlow

Ok(())
}
2 changes: 2 additions & 0 deletions rust/ql/test/query-tests/diagnostics/SummaryStats.expected
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@
| Inconsistencies - data flow | 0 |
| Lines of code extracted | 59 |
| Lines of user code extracted | 59 |
| Taint sources - active | 0 |
| Taint sources - total | 0 |