Skip to content

Commit c77eef3

Browse files
geoffw0geoffw0
committed
Rust: Convert the query to a path-problem with global data flow.
1 parent 209f394 commit c77eef3

File tree

5 files changed

+118
-23
lines changed

5 files changed

+118
-23
lines changed

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Provides classes and predicates for reasoning about disabled certificate
3+
* check vulnerabilities.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.FlowSink
9+
private import codeql.rust.Concepts
10+
11+
/**
12+
* Provides default sinks for detecting disabled certificate check
13+
* vulnerabilities, as well as extension points for adding your own.
14+
*/
15+
module DisabledCertificateCheckExtensions {
16+
/**
17+
* A data flow sink for disabled certificate check vulnerabilities.
18+
*/
19+
abstract class Sink extends QuerySink::Range {
20+
override string getSinkType() { result = "DisabledCertificateCheck" }
21+
}
22+
23+
/**
24+
* A default sink for disabled certificate check based on function names.
25+
*/
26+
private class DefaultSink extends Sink {
27+
DefaultSink() {
28+
exists(CallExprBase fc |
29+
fc.getStaticTarget().(Function).getName().getText() =
30+
["danger_accept_invalid_certs", "danger_accept_invalid_hostnames"] and
31+
fc.getArg(0) = this.asExpr().getExpr()
32+
)
33+
}
34+
}
35+
36+
/**
37+
* A sink for disabled certificate check from model data.
38+
*/
39+
private class ModelsAsDataSink extends Sink {
40+
ModelsAsDataSink() { sinkNode(this, "disable-certificate") }
41+
}
42+
}

‎rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Disabled TLS certificate check
33
* @description If an application disables TLS certificate checking, it may be vulnerable to
44
* man-in-the-middle attacks.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @security-severity 7.5
88
* @precision high
@@ -12,11 +12,31 @@
1212
*/
1313

1414
import rust
15+
import codeql.rust.dataflow.DataFlow
16+
import codeql.rust.security.DisabledCertificateCheckExtensions
1517

16-
from CallExprBase fc
17-
where
18-
fc.getStaticTarget().(Function).getName().getText() =
19-
["danger_accept_invalid_certs", "danger_accept_invalid_hostnames"] and
20-
fc.getArg(0).(BooleanLiteralExpr).getTextValue() = "true"
21-
select fc,
18+
/**
19+
* A taint configuration for disabling TLS certificate checks.
20+
*/
21+
module LogInjectionConfig implements DataFlow::ConfigSig {
22+
import DisabledCertificateCheckExtensions
23+
24+
predicate isSource(DataFlow::Node node) {
25+
node.asExpr().getExpr().(BooleanLiteralExpr).getTextValue() = "true"
26+
}
27+
28+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
29+
30+
predicate observeDiffInformedIncrementalMode() { any() }
31+
}
32+
33+
module DisabledCertificateCheckExtensionFlow = DataFlow::Global<LogInjectionConfig>;
34+
35+
import DisabledCertificateCheckExtensionFlow::PathGraph
36+
37+
from
38+
DisabledCertificateCheckExtensionFlow::PathNode sourceNode,
39+
DisabledCertificateCheckExtensionFlow::PathNode sinkNode
40+
where DisabledCertificateCheckExtensionFlow::flowPath(sourceNode, sinkNode)
41+
select sinkNode.getNode(), sourceNode, sinkNode,
2242
"Disabling TLS certificate validation can expose the application to man-in-the-middle attacks."

‎rust/ql/src/queries/summary/Stats.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ private import codeql.rust.security.AccessInvalidPointerExtensions
2222
private import codeql.rust.security.CleartextLoggingExtensions
2323
private import codeql.rust.security.CleartextStorageDatabaseExtensions
2424
private import codeql.rust.security.CleartextTransmissionExtensions
25+
private import codeql.rust.security.DisabledCertificateCheckExtensions
2526
private import codeql.rust.security.HardcodedCryptographicValueExtensions
2627
private import codeql.rust.security.InsecureCookieExtensions
2728
private import codeql.rust.security.LogInjectionExtensions

‎rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.expected‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,42 @@
1-
| main.rs:3:16:4:36 | ... .danger_accept_invalid_certs(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
2-
| main.rs:8:16:9:40 | ... .danger_accept_invalid_hostnames(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
3-
| main.rs:13:16:16:36 | ... .danger_accept_invalid_certs(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
4-
| main.rs:13:16:17:40 | ... .danger_accept_invalid_hostnames(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
5-
| main.rs:36:16:37:36 | ... .danger_accept_invalid_certs(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
6-
| main.rs:41:16:42:40 | ... .danger_accept_invalid_hostnames(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
7-
| main.rs:46:16:47:36 | ... .danger_accept_invalid_certs(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
8-
| main.rs:46:16:48:40 | ... .danger_accept_invalid_hostnames(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
9-
| main.rs:52:16:55:36 | ... .danger_accept_invalid_certs(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
10-
| main.rs:52:16:56:40 | ... .danger_accept_invalid_hostnames(...) | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
1+
#select
2+
| main.rs:4:32:4:35 | true | main.rs:4:32:4:35 | true | main.rs:4:32:4:35 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
3+
| main.rs:9:36:9:39 | true | main.rs:9:36:9:39 | true | main.rs:9:36:9:39 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
4+
| main.rs:16:32:16:35 | true | main.rs:16:32:16:35 | true | main.rs:16:32:16:35 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
5+
| main.rs:17:36:17:39 | true | main.rs:17:36:17:39 | true | main.rs:17:36:17:39 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
6+
| main.rs:37:32:37:35 | true | main.rs:37:32:37:35 | true | main.rs:37:32:37:35 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
7+
| main.rs:42:36:42:39 | true | main.rs:42:36:42:39 | true | main.rs:42:36:42:39 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
8+
| main.rs:47:32:47:35 | true | main.rs:47:32:47:35 | true | main.rs:47:32:47:35 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
9+
| main.rs:48:36:48:39 | true | main.rs:48:36:48:39 | true | main.rs:48:36:48:39 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
10+
| main.rs:55:32:55:35 | true | main.rs:55:32:55:35 | true | main.rs:55:32:55:35 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
11+
| main.rs:56:36:56:39 | true | main.rs:56:36:56:39 | true | main.rs:56:36:56:39 | true | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
12+
| main.rs:83:32:83:37 | always | main.rs:74:15:74:18 | true | main.rs:83:32:83:37 | always | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
13+
| main.rs:88:32:88:40 | sometimes | main.rs:75:22:75:25 | true | main.rs:88:32:88:40 | sometimes | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
14+
| main.rs:93:32:93:47 | sometimes_global | main.rs:106:17:106:20 | true | main.rs:93:32:93:47 | sometimes_global | Disabling TLS certificate validation can expose the application to man-in-the-middle attacks. |
15+
edges
16+
| main.rs:73:19:73:40 | ...: bool | main.rs:93:32:93:47 | sometimes_global | provenance | |
17+
| main.rs:74:6:74:11 | always | main.rs:83:32:83:37 | always | provenance | |
18+
| main.rs:74:15:74:18 | true | main.rs:74:6:74:11 | always | provenance | |
19+
| main.rs:75:6:75:18 | mut sometimes | main.rs:88:32:88:40 | sometimes | provenance | |
20+
| main.rs:75:22:75:25 | true | main.rs:75:6:75:18 | mut sometimes | provenance | |
21+
| main.rs:106:17:106:20 | true | main.rs:73:19:73:40 | ...: bool | provenance | |
22+
nodes
23+
| main.rs:4:32:4:35 | true | semmle.label | true |
24+
| main.rs:9:36:9:39 | true | semmle.label | true |
25+
| main.rs:16:32:16:35 | true | semmle.label | true |
26+
| main.rs:17:36:17:39 | true | semmle.label | true |
27+
| main.rs:37:32:37:35 | true | semmle.label | true |
28+
| main.rs:42:36:42:39 | true | semmle.label | true |
29+
| main.rs:47:32:47:35 | true | semmle.label | true |
30+
| main.rs:48:36:48:39 | true | semmle.label | true |
31+
| main.rs:55:32:55:35 | true | semmle.label | true |
32+
| main.rs:56:36:56:39 | true | semmle.label | true |
33+
| main.rs:73:19:73:40 | ...: bool | semmle.label | ...: bool |
34+
| main.rs:74:6:74:11 | always | semmle.label | always |
35+
| main.rs:74:15:74:18 | true | semmle.label | true |
36+
| main.rs:75:6:75:18 | mut sometimes | semmle.label | mut sometimes |
37+
| main.rs:75:22:75:25 | true | semmle.label | true |
38+
| main.rs:83:32:83:37 | always | semmle.label | always |
39+
| main.rs:88:32:88:40 | sometimes | semmle.label | sometimes |
40+
| main.rs:93:32:93:47 | sometimes_global | semmle.label | sometimes_global |
41+
| main.rs:106:17:106:20 | true | semmle.label | true |
42+
subpaths

‎rust/ql/test/query-tests/security/CWE-295/main.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,26 +71,26 @@ fn test_reqwest() {
7171
}
7272

7373
fn test_data_flow(sometimes_global: bool) {
74-
let always = true;
75-
let mut sometimes = true;
74+
let always = true; // $ Source=always
75+
let mut sometimes = true; // $ Source=sometimes
7676
let never = false;
7777

7878
if rand::random_range(0 .. 2) == 0 {
7979
sometimes = false;
8080
}
8181

8282
let _client = native_tls::TlsConnector::builder()
83-
.danger_accept_invalid_certs(always) // $ MISSING: Alert[rust/disabled-certificate-check]
83+
.danger_accept_invalid_certs(always) // $ Alert[rust/disabled-certificate-check]=always
8484
.build()
8585
.unwrap();
8686

8787
let _client = native_tls::TlsConnector::builder()
88-
.danger_accept_invalid_certs(sometimes) // $ MISSING: Alert[rust/disabled-certificate-check]
88+
.danger_accept_invalid_certs(sometimes) // $ Alert[rust/disabled-certificate-check]=sometimes
8989
.build()
9090
.unwrap();
9191

9292
let _client = native_tls::TlsConnector::builder()
93-
.danger_accept_invalid_certs(sometimes_global) // $ MISSING: Alert[rust/disabled-certificate-check]
93+
.danger_accept_invalid_certs(sometimes_global) // $ Alert[rust/disabled-certificate-check]=arg
9494
.build()
9595
.unwrap();
9696

@@ -103,6 +103,6 @@ fn test_data_flow(sometimes_global: bool) {
103103
fn main() {
104104
test_native_tls();
105105
test_reqwest();
106-
test_data_flow(true);
106+
test_data_flow(true); // $ Source=arg
107107
test_data_flow(false);
108108
}

0 commit comments

Comments
 (0)