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.

Sign up for GitHub

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

Jump to bottom

Rust: Sink models for rust/sql-injection #18129

Merged
merged 7 commits into from
Nov 29, 2024
+121 −59
Merged

Rust: Sink models for rust/sql-injection #18129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e96f15d
Rust: Add a test exposing SQL Injection sinks directly.
geoffw0 Nov 22, 2024
ba560f2
Rust: Model SQLx.
geoffw0 Nov 22, 2024
60c212b
Rust: Update for changes on main.
geoffw0 Nov 27, 2024
c113a0b
Rust: Fix typo.
geoffw0 Nov 27, 2024
2810d64
Rust: Fix ql-for-ql warning.
geoffw0 Nov 28, 2024
1d03384
Rust: Fix SqlExecute.
geoffw0 Nov 28, 2024
5b50a82
Rust: Clarify the doc on the two models a little.
geoffw0 Nov 28, 2024
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
6 changes: 3 additions & 3 deletions rust/ql/lib/codeql/rust/Concepts.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ module RemoteSource {
}

/**
* A data-flow node that constructs a SQL statement.
* A data-flow node that constructs a SQL statement (for later execution).
*
* Often, it is worthy of an alert if a SQL statement is constructed such that
* executing it would be a security risk.
Expand Down Expand Up @@ -133,10 +133,10 @@ module SqlConstruction {
}

/**
* A data-flow node that executes SQL statements.
* A data-flow node that constructs and executes SQL statements.
*
* If the context of interest is such that merely constructing a SQL statement
* would be valuable to report, consider using `SqlConstruction`.
* would be valuable to report, consider also using `SqlConstruction`.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `SqlExecution::Range` instead.
Expand Down
1 change: 1 addition & 0 deletions rust/ql/lib/codeql/rust/Frameworks.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

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

private import rust
private import codeql.rust.Concepts
private import codeql.rust.dataflow.DataFlow

/**
* A call to `sqlx::query` and variations.
*/
private class SqlxQuery extends SqlConstruction::Range {
CallExpr call;

SqlxQuery() {
this.asExpr().getExpr() = call and
call.getFunction().(PathExpr).getPath().getResolvedPath() =
[
"crate::query::query", "crate::query_as::query_as", "crate::query_with::query_with",
"crate::query_as_with::query_as_with", "crate::query_scalar::query_scalar",
"crate::query_scalar_with::query_scalar_with", "crate::raw_sql::raw_sql"
]
}

override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
}

/**
* A call to `sqlx::Executor::execute`.
*/
private class SqlxExecute extends SqlExecution::Range {
MethodCallExpr call;

SqlxExecute() {
this.asExpr().getExpr() = call and
call.(Resolvable).getResolvedPath() = "crate::executor::Executor::execute"
}

override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
}
2 changes: 2 additions & 0 deletions rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
testFailures
failures
19 changes: 19 additions & 0 deletions rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import rust
import codeql.rust.security.SqlInjectionExtensions
import utils.InlineExpectationsTest

module SqlSinksTest implements TestSig {
string getARelevantTag() { result = "sql-sink" }

predicate hasActualResult(Location location, string element, string tag, string value) {
exists(SqlInjection::Sink sink |
location = sink.getLocation() and
location.getFile().getBaseName() != "" and
element = sink.toString() and
tag = "sql-sink" and
value = ""
)
}
}

import MakeTest<SqlSinksTest>
112 changes: 56 additions & 56 deletions rust/ql/test/query-tests/security/CWE-089/sqlx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err

// construct queries (with extra variants)
let const_string = String::from("Alice");
let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING Source=args1
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote1
let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING: Source=args1
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote1
let remote_number = remote_string.parse::<i32>().unwrap_or(0);
let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='Alice'");
let safe_query_2 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
Expand All @@ -57,31 +57,31 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)

// direct execution
let _ = conn.execute(safe_query_1.as_str()).await?;
let _ = conn.execute(safe_query_2.as_str()).await?;
let _ = conn.execute(safe_query_3.as_str()).await?;
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=args1
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
if enable_remote {
let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
}

// prepared queries
let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?;
let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?;
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=args1
let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
if enable_remote {
let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
}
let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?; // $ sql-sink
if enable_remote {
let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?; // $ sql-sink
}

Ok(())
Expand All @@ -93,67 +93,67 @@ async fn test_sqlx_sqlite(url: &str, enable_remote: bool) -> Result<(), sqlx::Er

// construct queries
let const_string = String::from("Alice");
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote2
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote2
let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)

// direct execution (with extra variants)
let _ = conn.execute(safe_query_1.as_str()).await?;
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
if enable_remote {
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote2
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
}
// ...
let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?;
let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
if enable_remote {
let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
}

// prepared queries (with extra variants)
let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?;
let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?; // $ sql-sink
if enable_remote {
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?;
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?; // $ sql-sink
}
// ...
let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn);
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn);
let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn); // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn); // $ sql-sink
if enable_remote {
let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ MISSING Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn);
let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ sql-sink MISSING: Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn); // $ sql-sink
}
// ...
let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?;
let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink
println!(" row1 = {:?}", row1);
let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?;
let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?; // $ sql-sink
println!(" row2 = {:?}", row2);
if enable_remote {
let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?;
let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?; // $ sql-sink
}
// ...
let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data");
let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
println!(" row3 = {:?}", row3);
let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data");
let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
println!(" row4 = {:?}", row4);
if enable_remote {
let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING Alert[sql-injection]=remote2
let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data");
let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink $ MISSING: Alert[sql-injection]=remote2
let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
}
// ...
let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?;
let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?;
let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
if enable_remote {
let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?;
let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?;
let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
}
// ...
let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // (only takes string literals, so can't be vulnerable)
let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink (only takes string literals, so can't be vulnerable)
if enable_remote {
let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?;
let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
}

Ok(())
Expand All @@ -166,23 +166,23 @@ async fn test_sqlx_postgres(url: &str, enable_remote: bool) -> Result<(), sqlx::

// construct queries
let const_string = String::from("Alice");
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote3
let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote3
let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=$1"); // (prepared arguments are safe)

// direct execution
let _ = conn.execute(safe_query_1.as_str()).await?;
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
if enable_remote {
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote3
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
}

// prepared queries
let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?;
let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?; // $ sql-sink
if enable_remote {
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote3
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?;
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?; // $ sql-sink
}

Ok(())
Expand Down