Skip to content

Commit 7a18da8

Browse files
paldepindpaldepind
authored
Merge pull request #19000 from paldepind/rust-cleartext-transmission
Rust: Add cleartext transmission query
2 parents f90d53a + 0bf8265 commit 7a18da8

File tree

13 files changed

+362
-4
lines changed

13 files changed

+362
-4
lines changed

‎rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ extensions:
55
data:
66
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "crate::get", "ReturnValue.Field[crate::result::Result::Ok(0)]", "remote", "manual"]
77
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "crate::blocking::get", "ReturnValue.Field[crate::result::Result::Ok(0)]", "remote", "manual"]
8+
- addsTo:
9+
pack: codeql/rust-all
10+
extensible: sinkModel
11+
data:
12+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::request", "Argument[1]", "transmission", "manual"]
13+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::request", "Argument[1]", "transmission", "manual"]
814
- addsTo:
915
pack: codeql/rust-all
1016
extensible: summaryModel

‎rust/ql/lib/codeql/rust/frameworks/url.model.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Models for the `url` crate
2+
extensions:
3+
- addsTo:
4+
pack: codeql/rust-all
5+
extensible: summaryModel
6+
data:
7+
- ["repo:https://github.com/servo/rust-url:url", "<crate::Url>::parse", "Argument[0].Reference", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"]

‎rust/ql/lib/codeql/rust/security/CleartextTransmissionExtensions.qll

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cleartext transmission
3+
* vulnerabilities.
4+
*/
5+
6+
private import codeql.util.Unit
7+
private import rust
8+
private import codeql.rust.dataflow.DataFlow
9+
private import codeql.rust.dataflow.FlowSink
10+
11+
/**
12+
* A data flow sink for cleartext transmission vulnerabilities. That is,
13+
* a `DataFlow::Node` of something that is transmitted over a network.
14+
*/
15+
abstract class CleartextTransmissionSink extends DataFlow::Node { }
16+
17+
/**
18+
* A barrier for cleartext transmission vulnerabilities.
19+
*/
20+
abstract class CleartextTransmissionBarrier extends DataFlow::Node { }
21+
22+
/**
23+
* A unit class for adding additional flow steps.
24+
*/
25+
class CleartextTransmissionAdditionalFlowStep extends Unit {
26+
/**
27+
* Holds if the step from `node1` to `node2` should be considered a flow
28+
* step for paths related to cleartext transmission vulnerabilities.
29+
*/
30+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
31+
}
32+
33+
/**
34+
* A sink defined through MaD.
35+
*/
36+
private class MadCleartextTransmissionSink extends CleartextTransmissionSink {
37+
MadCleartextTransmissionSink() { sinkNode(this, "transmission") }
38+
}

‎rust/ql/lib/ext/generated/reqwest/repo-https-github.com-seanmonstar-reqwest-reqwest.model.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# THIS FILE IS AN AUTO-GENERATED MODELS AS DATA FILE. DO NOT EDIT.
2+
extensions:
3+
- addsTo:
4+
pack: codeql/rust-all
5+
extensible: sinkModel
6+
data:
7+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::delete", "Argument[0]", "transmission", "df-generated"]
8+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::get", "Argument[0]", "transmission", "df-generated"]
9+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::head", "Argument[0]", "transmission", "df-generated"]
10+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::patch", "Argument[0]", "transmission", "df-generated"]
11+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::post", "Argument[0]", "transmission", "df-generated"]
12+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::async_impl::client::Client>::put", "Argument[0]", "transmission", "df-generated"]
13+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::delete", "Argument[0]", "transmission", "df-generated"]
14+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::get", "Argument[0]", "transmission", "df-generated"]
15+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::head", "Argument[0]", "transmission", "df-generated"]
16+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::patch", "Argument[0]", "transmission", "df-generated"]
17+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::post", "Argument[0]", "transmission", "df-generated"]
18+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::client::Client>::put", "Argument[0]", "transmission", "df-generated"]
19+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::connect::Connector as crate::Service>::call", "Argument[0]", "log-injection", "df-generated"]
20+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::connect::ConnectorService as crate::Service>::call", "Argument[0]", "log-injection", "df-generated"]
21+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "crate::blocking::get", "Argument[0]", "transmission", "df-generated"]
22+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "crate::blocking::wait::timeout", "Argument[1]", "log-injection", "df-generated"]
23+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "crate::get", "Argument[0]", "transmission", "df-generated"]

‎rust/ql/src/queries/security/CWE-311/CleartextTransmission.qhelp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
5+
<p>
6+
Sensitive information that is transmitted without encryption may be accessible
7+
to an attacker.
8+
</p>
9+
10+
</overview>
11+
<recommendation>
12+
13+
<p>
14+
Ensure that sensitive information is always encrypted before being transmitted
15+
over the network. In general, decrypt sensitive information only at the point
16+
where it is necessary for it to be used in cleartext. Avoid transmitting
17+
sensitive information when it is not necessary to.
18+
</p>
19+
20+
</recommendation>
21+
<example>
22+
23+
<p>
24+
The following example shows three cases of transmitting information. In the
25+
'BAD' case, the transmitted data is sensitive (a credit card number) and is
26+
included as cleartext in the URL. URLs are often logged or otherwise visible in
27+
cleartext, and should not contain sensitive information.
28+
</p>
29+
30+
<p>
31+
In the 'GOOD' cases, the data is either not sensitive, or is protected with
32+
encryption. When encryption is used, ensure that you select a secure modern
33+
encryption algorithm, and put suitable key management practices into place.
34+
</p>
35+
36+
<sample src="CleartextTransmission.rs" />
37+
38+
</example>
39+
<references>
40+
41+
<li>
42+
OWASP Top 10:2021:
43+
<a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">A02:2021 - Cryptographic Failures</a>.
44+
</li>
45+
<li>
46+
OWASP:
47+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Key_Management_Cheat_Sheet.html">Key Management Cheat Sheet</a>.
48+
</li>
49+
50+
</references>
51+
</qhelp>

‎rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @name Cleartext transmission of sensitive information
3+
* @description Transmitting sensitive information across a network in
4+
* cleartext can expose it to an attacker.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id rust/cleartext-transmission
10+
* @tags security
11+
* external/cwe/cwe-319
12+
*/
13+
14+
import rust
15+
import codeql.rust.dataflow.DataFlow
16+
import codeql.rust.security.SensitiveData
17+
import codeql.rust.dataflow.TaintTracking
18+
import codeql.rust.security.CleartextTransmissionExtensions
19+
20+
/**
21+
* A taint configuration from sensitive information to expressions that are
22+
* transmitted over a network.
23+
*/
24+
module CleartextTransmissionConfig implements DataFlow::ConfigSig {
25+
predicate isSource(DataFlow::Node node) { node instanceof SensitiveData }
26+
27+
predicate isSink(DataFlow::Node node) { node instanceof CleartextTransmissionSink }
28+
29+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof CleartextTransmissionBarrier }
30+
31+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
32+
any(CleartextTransmissionAdditionalFlowStep s).step(nodeFrom, nodeTo)
33+
}
34+
35+
predicate isBarrierIn(DataFlow::Node node) {
36+
// make sources barriers so that we only report the closest instance
37+
isSource(node)
38+
}
39+
}
40+
41+
module CleartextTransmissionFlow = TaintTracking::Global<CleartextTransmissionConfig>;
42+
43+
import CleartextTransmissionFlow::PathGraph
44+
45+
from CleartextTransmissionFlow::PathNode sourceNode, CleartextTransmissionFlow::PathNode sinkNode
46+
where CleartextTransmissionFlow::flowPath(sourceNode, sinkNode)
47+
select sinkNode.getNode(), sourceNode, sinkNode,
48+
"The operation '" + sinkNode.getNode().toString() +
49+
"', transmits data which may contain unencrypted sensitive data from $@.", sourceNode,
50+
sourceNode.getNode().toString()

‎rust/ql/src/queries/security/CWE-311/CleartextTransmission.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
func getData() {
2+
// ...
3+
4+
// GOOD: not sensitive information
5+
let body = reqwest::get("https://example.com/song/{faveSong}").await?.text().await?;
6+
7+
// BAD: sensitive information sent in cleartext in the URL
8+
let body = reqwest::get(format!("https://example.com/card/{creditCardNo}")).await?.text().await?;
9+
10+
// GOOD: encrypted sensitive information sent in the URL
11+
let encryptedPassword = encrypt(password, encryptionKey);
12+
let body = reqwest::get(format!("https://example.com/card/{creditCardNo}")).await?.text().await?;
13+
14+
// ...
15+
}

‎rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,6 +2216,7 @@ storeStep
22162216
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::chunk | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::chunk |
22172217
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::text | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::text |
22182218
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::text_with_charset | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::response::Response>::text_with_charset |
2219+
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in repo:https://github.com/servo/rust-url:url::_::<crate::Url>::parse | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/servo/rust-url:url::_::<crate::Url>::parse |
22192220
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)].Field[0] in lang:alloc::_::<crate::collections::btree::node::NodeRef>::search_tree_for_bifurcation | tuple.0 | file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in lang:alloc::_::<crate::collections::btree::node::NodeRef>::search_tree_for_bifurcation |
22202221
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)].Field[0] in lang:std::_::<crate::sync::poison::condvar::Condvar>::wait_timeout | tuple.0 | file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in lang:std::_::<crate::sync::poison::condvar::Condvar>::wait_timeout |
22212222
| file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)].Field[0] in lang:std::_::<crate::sync::poison::condvar::Condvar>::wait_timeout_ms | tuple.0 | file://:0:0:0:0 | [summary] to write: ReturnValue.Field[crate::result::Result::Ok(0)] in lang:std::_::<crate::sync::poison::condvar::Condvar>::wait_timeout_ms |
@@ -2495,6 +2496,7 @@ readStep
24952496
| file://:0:0:0:0 | [summary param] 0 in lang:std::_::crate::thread::current::try_with_current | function return | file://:0:0:0:0 | [summary] read: Argument[0].ReturnValue in lang:std::_::crate::thread::current::try_with_current |
24962497
| file://:0:0:0:0 | [summary param] 0 in lang:std::_::crate::thread::with_current_name | function return | file://:0:0:0:0 | [summary] read: Argument[0].ReturnValue in lang:std::_::crate::thread::with_current_name |
24972498
| file://:0:0:0:0 | [summary param] 0 in repo:https://github.com/rust-lang/regex:regex::_::crate::escape | &ref | file://:0:0:0:0 | [summary] read: Argument[0].Reference in repo:https://github.com/rust-lang/regex:regex::_::crate::escape |
2499+
| file://:0:0:0:0 | [summary param] 0 in repo:https://github.com/servo/rust-url:url::_::<crate::Url>::parse | &ref | file://:0:0:0:0 | [summary] read: Argument[0].Reference in repo:https://github.com/servo/rust-url:url::_::<crate::Url>::parse |
24982500
| file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::<crate::vec::into_iter::IntoIter as crate::iter::traits::iterator::Iterator>::fold | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::<crate::vec::into_iter::IntoIter as crate::iter::traits::iterator::Iterator>::fold |
24992501
| file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::crate::collections::btree::mem::replace | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::crate::collections::btree::mem::replace |
25002502
| file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::crate::collections::btree::mem::take_mut | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::crate::collections::btree::mem::take_mut |

‎rust/ql/test/query-tests/security/CWE-020/RegexInjection.expected

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22
| main.rs:6:25:6:30 | &regex | main.rs:4:20:4:32 | ...::var | main.rs:6:25:6:30 | &regex | This regular expression is constructed from a $@. | main.rs:4:20:4:32 | ...::var | user-provided value |
33
edges
44
| main.rs:4:9:4:16 | username | main.rs:5:25:5:44 | MacroExpr | provenance | |
5-
| main.rs:4:20:4:32 | ...::var | main.rs:4:20:4:40 | ...::var(...) [Ok] | provenance | Src:MaD:60 |
6-
| main.rs:4:20:4:40 | ...::var(...) [Ok] | main.rs:4:20:4:66 | ... .unwrap_or(...) | provenance | MaD:1573 |
5+
| main.rs:4:20:4:32 | ...::var | main.rs:4:20:4:40 | ...::var(...) [Ok] | provenance | Src:MaD:62 |
6+
| main.rs:4:20:4:40 | ...::var(...) [Ok] | main.rs:4:20:4:66 | ... .unwrap_or(...) | provenance | MaD:1593 |
77
| main.rs:4:20:4:66 | ... .unwrap_or(...) | main.rs:4:9:4:16 | username | provenance | |
88
| main.rs:5:9:5:13 | regex | main.rs:6:26:6:30 | regex | provenance | |
99
| main.rs:5:17:5:45 | res | main.rs:5:25:5:44 | { ... } | provenance | |
1010
| main.rs:5:25:5:44 | ...::format(...) | main.rs:5:17:5:45 | res | provenance | |
1111
| main.rs:5:25:5:44 | ...::must_use(...) | main.rs:5:9:5:13 | regex | provenance | |
12-
| main.rs:5:25:5:44 | MacroExpr | main.rs:5:25:5:44 | ...::format(...) | provenance | MaD:64 |
13-
| main.rs:5:25:5:44 | { ... } | main.rs:5:25:5:44 | ...::must_use(...) | provenance | MaD:2996 |
12+
| main.rs:5:25:5:44 | MacroExpr | main.rs:5:25:5:44 | ...::format(...) | provenance | MaD:66 |
13+
| main.rs:5:25:5:44 | { ... } | main.rs:5:25:5:44 | ...::must_use(...) | provenance | MaD:3016 |
1414
| main.rs:6:26:6:30 | regex | main.rs:6:25:6:30 | &regex | provenance | |
1515
nodes
1616
| main.rs:4:9:4:16 | username | semmle.label | username |

0 commit comments

Comments
 (0)