Skip to content

Rust: Model futures-io, rustls, futures-rustls #19626

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/async-rs.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/rust-all
extensible: sourceModel
data:
- ["repo:https://github.com/async-rs/async-std:async-std", "<crate::net::tcp::stream::TcpStream>::connect", "ReturnValue.Future.Field[crate::result::Result::Ok(0)]", "remote", "manual"]
13 changes: 13 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/futures.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,16 @@ extensions:
extensible: summaryModel
data:
- ["repo:https://github.com/rust-lang/futures-rs:futures-executor", "crate::local_pool::block_on", "Argument[0]", "ReturnValue", "value", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "<crate::io::buf_reader::BufReader>::new", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read_to_end", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read_to_end", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_line", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_line", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_until", "Argument[self]", "Argument[1].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_until", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
Comment on lines +8 to +15
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

[nitpick] These two read rules only differ by .Reference. Consider consolidating into a single pattern (e.g., match Argument[self]) to reduce duplication and maintenance overhead.

Suggested change
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read_to_end", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read_to_end", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_line", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_line", "Argument[self].Reference", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_until", "Argument[self]", "Argument[1].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_until", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read", "Argument[self].*", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncReadExt::read_to_end", "Argument[self].*", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_line", "Argument[self].*", "Argument[0].Reference", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::read_until", "Argument[self].*", "Argument[1].Reference", "taint", "manual"]

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's missing data flow through certain implicit dereferences, when that's fixed we can get rid of the duplicate models.

- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::fill_buf", "Argument[self]", "ReturnValue.Future.Field[crate::result::Result::Ok(0)]", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::io::AsyncBufReadExt::lines", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "crate::stream::stream::StreamExt::next", "Argument[self]", "ReturnValue.Future.Field[crate::option::Option::Some(0)]", "taint", "manual"]
- ["repo:https://github.com/rust-lang/futures-rs:futures-util", "<crate::io::buf_reader::BufReader as crate::if_std::AsyncBufRead>::poll_fill_buf", "Argument[self].Reference", "ReturnValue.Field[crate::task::poll::Poll::Ready(0)].Field[crate::result::Result::Ok(0)]", "taint", "manual"]
14 changes: 14 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/rustls.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
extensions:
- addsTo:
pack: codeql/rust-all
extensible: sourceModel
data:
- ["repo:https://github.com/rustls/rustls:rustls", "<crate::client::client_conn::connection::ClientConnection>::new", "ReturnValue.Field[crate::result::Result::Ok(0)]", "remote", "manual"]
- addsTo:
pack: codeql/rust-all
extensible: summaryModel
data:
- ["repo:https://github.com/quininer/futures-rustls:futures-rustls", "<crate::TlsConnector>::connect", "Argument[1]", "ReturnValue.Future.Field[crate::result::Result::Ok(0)]", "taint", "manual"]
- ["repo:https://github.com/quininer/futures-rustls:futures-rustls", "<crate::client::TlsStream as crate::if_std::AsyncRead>::poll_read", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

Add a summaryModel entry for <crate::client::TlsStream as crate::if_std::AsyncBufRead>::poll_fill_buf so that taint is correctly propagated through buffered reads, mirroring the existing poll_read rule.

Suggested change
- ["repo:https://github.com/quininer/futures-rustls:futures-rustls", "<crate::client::TlsStream as crate::if_std::AsyncRead>::poll_read", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
- ["repo:https://github.com/quininer/futures-rustls:futures-rustls", "<crate::client::TlsStream as crate::if_std::AsyncRead>::poll_read", "Argument[self].Reference", "Argument[1].Reference", "taint", "manual"]
- ["repo:https://github.com/quininer/futures-rustls:futures-rustls", "<crate::client::TlsStream as crate::if_std::AsyncBufRead>::poll_fill_buf", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice try, but it doesn't help. Hopefully when we have support for MaD trait models, we can clean this stuff up.

- ["repo:https://github.com/rustls/rustls:rustls", "<crate::conn::ConnectionCommon>::reader", "Argument[self]", "ReturnValue", "taint", "manual"]
- ["repo:https://github.com/rustls/rustls:rustls", "<crate::conn::connection::Reader as crate::io::Read>::read", "Argument[self]", "Argument[0].Reference", "taint", "manual"]
Original file line number Diff line number Diff line change
@@ -1,3 +1,25 @@
multipleMethodCallTargets
| test.rs:618:25:618:49 | address.to_socket_addrs() | file://:0:0:0:0 | fn to_socket_addrs |
| test.rs:618:25:618:49 | address.to_socket_addrs() | file://:0:0:0:0 | fn to_socket_addrs |
| test_futures_io.rs:35:26:35:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:35:26:35:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:35:26:35:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:61:22:61:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
| test_futures_io.rs:61:22:61:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
| test_futures_io.rs:68:23:68:67 | ... .poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
| test_futures_io.rs:68:23:68:67 | ... .poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
| test_futures_io.rs:92:26:92:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:92:26:92:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:92:26:92:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
| test_futures_io.rs:115:22:115:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
| test_futures_io.rs:115:22:115:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
multiplePathResolutions
| test.rs:777:23:777:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
| test.rs:777:23:777:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
| test.rs:777:23:777:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
| test_futures_io.rs:25:23:25:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
| test_futures_io.rs:25:23:25:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
| test_futures_io.rs:25:23:25:61 | ...::try_from | file://:0:0:0:0 | fn try_from |
multipleCanonicalPaths
| file://:0:0:0:0 | fn to_ordering | file://:0:0:0:0 | Crate([email protected]) | <typenum::Equal as core::cmp::Ord>::to_ordering |
| file://:0:0:0:0 | fn to_ordering | file://:0:0:0:0 | Crate([email protected]) | <typenum::Equal as typenum::marker_traits::Ord>::to_ordering |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module MyFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource }

predicate isSink(DataFlow::Node sink) {
any(CallExpr call | call.getFunction().(PathExpr).getResolvedPath() = "crate::test::sink")
any(CallExpr call | call.getFunction().(PathExpr).getResolvedPath().matches("%::sink"))
.getArgList()
.getAnArg() = sink.asExpr().getExpr()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@
| test.rs:619:26:619:61 | ...::connect_timeout | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:671:28:671:57 | ...::connect | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:753:22:753:49 | ...::connect | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:775:16:775:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). |
| test.rs:775:16:775:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). |
| test.rs:779:22:779:50 | ...::new | Flow source 'RemoteSource' of type remote (DEFAULT). |
| test.rs:806:16:806:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). |
| test.rs:806:16:806:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). |
| test_futures_io.rs:19:15:19:32 | ...::connect | Flow source 'RemoteSource' of type remote (DEFAULT). |
| web_frameworks.rs:12:31:12:31 | a | Flow source 'RemoteSource' of type remote (DEFAULT). |
| web_frameworks.rs:12:31:12:31 | a | Flow source 'RemoteSource' of type remote (DEFAULT). |
| web_frameworks.rs:21:31:21:35 | TuplePat | Flow source 'RemoteSource' of type remote (DEFAULT). |
Expand Down
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
Expand Up @@ -13,3 +13,6 @@ qltest_dependencies:
- actix-web = { version = "4.10.2" }
- axum = { version = "0.8.4" }
- serde_json = { version = "1.0.140" }
- rustls = { version = "0.23.27" }
- futures-rustls = { version = "0.26.0" }
- async-std = { version = "1.13.1" }
37 changes: 37 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,37 @@ async fn test_std_to_tokio_tcpstream() -> std::io::Result<()> {
Ok(())
}

fn test_rustls() -> std::io::Result<()> {
let config = rustls::ClientConfig::builder()
.with_root_certificates(rustls::RootCertStore::empty())
.with_no_client_auth();
let server_name = rustls::pki_types::ServerName::try_from("www.example.com").unwrap();
let config_arc = std::sync::Arc::new(config);
let mut client = rustls::ClientConnection::new(config_arc, server_name).unwrap(); // $ Alert[rust/summary/taint-sources]
let mut reader = client.reader();
sink(&reader); // $ hasTaintFlow=config_arc

{
let mut buffer = [0u8; 100];
let _bytes = reader.read(&mut buffer)?;
sink(&buffer); // $ hasTaintFlow=config_arc
}

{
let mut buffer = Vec::<u8>::new();
let _bytes = reader.read_to_end(&mut buffer)?;
sink(&buffer); // $ hasTaintFlow=config_arc
}

{
let mut buffer = String::new();
let _bytes = reader.read_to_string(&mut buffer)?;
sink(&buffer); // $ hasTaintFlow=config_arc
}

Ok(())
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let case = std::env::args().nth(1).unwrap_or(String::from("1")).parse::<i64>().unwrap(); // $ Alert[rust/summary/taint-sources]
Expand Down Expand Up @@ -849,5 +880,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
Err(e) => println!("error: {}", e),
}

println!("test_rustls...");
match test_rustls() {
Ok(_) => println!("complete"),
Err(e) => println!("error: {}", e),
}

Ok(())
}
159 changes: 159 additions & 0 deletions rust/ql/test/library-tests/dataflow/sources/test_futures_io.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
fn sink<T>(_: T) { }

// --- tests ---

use std::pin::Pin;
use std::task::{Context, Poll};
use std::io;
use futures::io::AsyncRead;
use futures::io::AsyncReadExt;
use futures::io::AsyncBufRead;
use futures::io::AsyncBufReadExt;
use futures::StreamExt;
use futures_rustls::{TlsConnector};
use async_std::sync::Arc;
use async_std::net::TcpStream;

async fn test_futures_rustls_futures_io() -> io::Result<()> {
let url = "www.example.com:443";
let tcp = TcpStream::connect(url).await?; // $ Alert[rust/summary/taint-sources]
sink(&tcp); // $ hasTaintFlow=url
let config = rustls::ClientConfig::builder()
.with_root_certificates(rustls::RootCertStore::empty())
.with_no_client_auth();
let connector = TlsConnector::from(Arc::new(config));
let server_name = rustls::pki_types::ServerName::try_from("www.example.com").unwrap();
let mut reader = connector.connect(server_name, tcp).await?;
sink(&reader); // $ hasTaintFlow=url

{
// using the `AsyncRead` trait (low-level)
let mut buffer = [0u8; 64];
let mut pinned = Pin::new(&mut reader);
sink(&pinned); // $ hasTaintFlow=url
let mut cx = Context::from_waker(futures::task::noop_waker_ref());
let bytes_read = pinned.poll_read(&mut cx, &mut buffer);
if let Poll::Ready(Ok(n)) = bytes_read {
sink(&buffer); // $ hasTaintFlow=url
sink(&buffer[..n]); // $ hasTaintFlow=url
}
}

{
// using the `AsyncReadExt::read` extension method (higher-level)
let mut buffer1 = [0u8; 64];
let bytes_read1 = futures::io::AsyncReadExt::read(&mut reader, &mut buffer1).await?;
sink(&buffer1[..bytes_read1]); // $ hasTaintFlow=url

let mut buffer2 = [0u8; 64];
let bytes_read2 = reader.read(&mut buffer2).await?;
sink(&buffer2[..bytes_read2]); // $ hasTaintFlow=url
}

let mut reader2 = futures::io::BufReader::new(reader);
sink(&reader2); // $ hasTaintFlow=url

{
// using the `AsyncBufRead` trait (low-level)
let mut pinned = Pin::new(&mut reader2);
sink(&pinned); // $ hasTaintFlow=url
let mut cx = Context::from_waker(futures::task::noop_waker_ref());
let buffer = pinned.poll_fill_buf(&mut cx);
if let Poll::Ready(Ok(buf)) = buffer {
sink(&buffer); // $ MISSING: hasTaintFlow=url
sink(buf); // $ MISSING: hasTaintFlow=url
}

// using the `AsyncBufRead` trait (alternative syntax)
let buffer2 = Pin::new(&mut reader2).poll_fill_buf(&mut cx);
match (buffer2) {
Poll::Ready(Ok(buf)) => {
sink(&buffer2); // $ MISSING: hasTaintFlow=url
sink(buf); // $ MISSING: hasTaintFlow=url
}
_ => {
// ...
}
}
}

{
// using the `AsyncBufReadExt::fill_buf` extension method (higher-level)
let buffer = reader2.fill_buf().await?;
sink(buffer); // $ hasTaintFlow=url
}

{
// using the `AsyncRead` trait (low-level)
let mut buffer = [0u8; 64];
let mut pinned = Pin::new(&mut reader2);
sink(&pinned); // $ hasTaintFlow=url
let mut cx = Context::from_waker(futures::task::noop_waker_ref());
let bytes_read = pinned.poll_read(&mut cx, &mut buffer);
sink(&buffer); // $ MISSING: hasTaintFlow=url
if let Poll::Ready(Ok(n)) = bytes_read {
sink(&buffer[..n]); // $ MISSING: hasTaintFlow=url
}
}

{
// using the `AsyncReadExt::read` extension method (higher-level)
let mut buffer1 = [0u8; 64];
let bytes_read1 = futures::io::AsyncReadExt::read(&mut reader2, &mut buffer1).await?;
sink(&buffer1[..bytes_read1]); // $ hasTaintFlow=url

let mut buffer2 = [0u8; 64];
let bytes_read2 = reader2.read(&mut buffer2).await?;
sink(&buffer2[..bytes_read2]); // $ hasTaintFlow=url
}

{
// using the `AsyncBufRead` trait (low-level)
let mut pinned = Pin::new(&mut reader2);
sink(&pinned); // $ hasTaintFlow=url
let mut cx = Context::from_waker(futures::task::noop_waker_ref());
let buffer = pinned.poll_fill_buf(&mut cx);
sink(&buffer); // $ MISSING: hasTaintFlow=url
if let Poll::Ready(Ok(buf)) = buffer {
sink(buf); // $ MISSING: hasTaintFlow=url
}
}

{
// using the `AsyncBufReadExt::fill_buf` extension method (higher-level)
let buffer = reader2.fill_buf().await?;
sink(buffer); // $ hasTaintFlow=url
}

{
// using the `AsyncBufReadExt::read_until` extension method
let mut line = Vec::new();
let _bytes_read = reader2.read_until(b'\n', &mut line).await?;
sink(&line); // $ hasTaintFlow=url
}

{
// using the `AsyncBufReadExt::read_line` extension method
let mut line = String::new();
let _bytes_read = reader2.read_line(&mut line).await?;
sink(&line); // $ hasTaintFlow=url
}

{
// using the `AsyncBufReadExt::read_to_end` extension method
let mut buffer = Vec::with_capacity(1024);
let _bytes_read = reader2.read_to_end(&mut buffer).await?;
sink(&buffer); // $ hasTaintFlow=url
}

{
// using the `AsyncBufReadExt::lines` extension method
let mut lines_stream = reader2.lines();
sink(lines_stream.next().await.unwrap()); // $ hasTaintFlow=url
while let Some(line) = lines_stream.next().await {
sink(line.unwrap()); // $ MISSING: hasTaintFlow
}
}

Ok(())
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ mod poem_test {

#[handler]
fn my_poem_handler_1(Path(a): Path<String>) -> String { // $ Alert[rust/summary/taint-sources]
sink(a.as_str()); // $ MISSING: hasTaintFlow
sink(a.as_bytes()); // $ MISSING: hasTaintFlow
sink(a); // $ MISSING: hasTaintFlow
sink(a.as_str()); // $ hasTaintFlow
sink(a.as_bytes()); // $ hasTaintFlow
sink(a); // $ hasTaintFlow

"".to_string()
}
Expand Down Expand Up @@ -59,7 +59,7 @@ mod poem_test {
fn my_poem_handler_6(
Query(a): Query<String>, // $ Alert[rust/summary/taint-sources]
) -> String {
sink(a); // $ MISSING: hasTaintFlow
sink(a); // $ hasTaintFlow

"".to_string()
}
Expand Down