Rust: Model futures-io, rustls, futures-rustls by geoffw0 · Pull Request #19626 · github/codeql

geoffw0 · 2025-05-29T16:03:39Z

Add models for futures-io (traits, part of futures-rs), and for rustls + futures-rustls (which uses the traits and includes what seem like high value sources).

Most of these work well, but the models for poll_read and poll_fill_buf aren't working reliably. I haven't been able to figure out what's wrong and I suspect it isn't an issue with the models themselves.

There's also a fix of the test sink recognition logic that allows the test to find sinks in sources/web_frameworks.rs, which also gives us a few results in sources/web_frameworks.rs that we should have been finding all along.

…g (reference). We shouldn't need these.

Copilot

Pull Request Overview

This PR adds CodeQL models and tests to improve taint-tracking for asynchronous I/O in Rust, covering futures-io, rustls, and futures-rustls. It also fixes test sink recognition in the web_frameworks suite.

Introduce test_futures_io.rs and test_rustls in test.rs to exercise AsyncRead/AsyncBufRead patterns over TLS streams.
Add dependency entries for rustls, futures-rustls, and async-std in options.yml.
Provide new model YAMLs (rustls.model.yml, futures.model.yml, async-rs.model.yml) to define taint sources and summaries for the relevant crates.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rust/ql/test/library-tests/dataflow/sources/web_frameworks.rs	Update expected sink annotations from `$ MISSING` to `$ hasTaintFlow`.
rust/ql/test/library-tests/dataflow/sources/test_futures_io.rs	New tests covering `AsyncRead`/`AsyncBufRead` over TLS streams.
rust/ql/test/library-tests/dataflow/sources/test.rs	Add `test_rustls` function and invoke it from `main`.
rust/ql/test/library-tests/dataflow/sources/options.yml	Add `rustls`, `futures-rustls`, and `async-std` dependencies.
rust/ql/test/library-tests/dataflow/sources/TaintSources.expected	Update expected taint alerts for new connection and args sources.
rust/ql/test/library-tests/dataflow/sources/InlineFlow.ql	Broaden sink matching predicate to `%::sink`.
rust/ql/lib/codeql/rust/frameworks/rustls.model.yml	Add source/summary model entries for `rustls` and `futures-rustls`.
rust/ql/lib/codeql/rust/frameworks/futures.model.yml	Add summary model entries for `futures-util` I/O and streams.
rust/ql/lib/codeql/rust/frameworks/async-rs.model.yml	Add source model entry for `async-std` TCP connect.

rust/ql/test/library-tests/dataflow/sources/TaintSources.expected

Copilot · 2025-05-29T16:06:55Z

rust/ql/lib/codeql/rust/frameworks/rustls.model.yml

+      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"]


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"]

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

Copilot · 2025-05-29T16:06:55Z

rust/ql/lib/codeql/rust/frameworks/futures.model.yml

+      - ["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"]


[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"]

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

paldepind

Looks good to me. I've created an internal issue for the problem about the same model not working for both function calls and method calls.

In reader.read(&mut buffer2) there is an implicit borrow of reader, so this is equivalent to (&reader).read(&mut buffer2). Therefore I think that the model that we actually want is the ones from 4d51a15 with Argument[self].Reference and not the ones with Argument[self].

geoffw0 · 2025-06-10T12:54:01Z

I've created an internal issue for the problem about the same model not working for both function calls and method calls.

Thanks, that's fantastic.

I've just merged in a recent main, it looks that will make the consistency check failure go away...

geoffw0 · 2025-06-10T13:48:04Z

It turns out I needed an even more recent main than my first attempt. The consistency check failure / merge conflict is now fixed.

Ready for re-approval and merge.

geoffw0 · 2025-06-10T14:23:40Z

Thanks!

geoffw0 added 7 commits May 29, 2025 16:45

Rust: Add tests for sources involving futures-rustls and futures-io.

544af7f

Rust: Add tests for sources involving regular rustls as well.

a5e1702

Rust: Fix a bug in InlineFlow.ql that was excluding some sinks.

b78d51e

Rust: Add source / basic basic models.

13f6de9

Rust: Add models for read methods.

84c72f6

Rust: Add model variants for when the qualifier is expressed as an ar…

4d51a15

…g (reference). We shouldn't need these.

Rust: Model more methods.

10f894b

Copilot AI review requested due to automatic review settings May 29, 2025 16:03

geoffw0 requested a review from a team as a code owner May 29, 2025 16:03

geoffw0 added no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code labels May 29, 2025

Copilot AI reviewed May 29, 2025

View reviewed changes

Rust: Accept consistency test failures.

49dabdb

paldepind previously approved these changes Jun 10, 2025

View reviewed changes

Merge branch 'main' into futures

918fa36

Merge branch 'main' into futures

fd2154d

geoffw0 dismissed paldepind’s stale review via fd2154d June 10, 2025 13:16

paldepind approved these changes Jun 10, 2025

View reviewed changes

geoffw0 merged commit 652d32d into github:main Jun 10, 2025
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Rust: Model futures-io, rustls, futures-rustls#19626
geoffw0 merged 10 commits intogithub:mainfrom
geoffw0:futures

geoffw0 commented May 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI May 29, 2025

Uh oh!

geoffw0 May 29, 2025

Uh oh!

Copilot AI May 29, 2025

Uh oh!

geoffw0 May 29, 2025

Uh oh!

paldepind left a comment

Uh oh!

geoffw0 commented Jun 10, 2025

Uh oh!

geoffw0 commented Jun 10, 2025 •

edited

Loading

Uh oh!

geoffw0 commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	- ["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"]

Conversation

geoffw0 commented May 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI May 29, 2025

Choose a reason for hiding this comment

Uh oh!

geoffw0 May 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 29, 2025

Choose a reason for hiding this comment

Uh oh!

geoffw0 May 29, 2025

Choose a reason for hiding this comment

Uh oh!

paldepind left a comment

Choose a reason for hiding this comment

Uh oh!

geoffw0 commented Jun 10, 2025

Uh oh!

geoffw0 commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geoffw0 commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

geoffw0 commented Jun 10, 2025 •

edited

Loading