Skip to content

Commit f32fd38

Browse files
geoffw0geoffw0
authored
Merge pull request #18582 from geoffw0/logging
Rust: Query for cleartext logging of sensitive information
2 parents 1bacb99 + 0a3d44c commit f32fd38

File tree

20 files changed

+830
-30
lines changed

20 files changed

+830
-30
lines changed

‎rust/ql/integration-tests/hello-project/summary.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
| Macro calls - resolved | 2 |
1515
| Macro calls - total | 2 |
1616
| Macro calls - unresolved | 0 |
17-
| Taint edges - number of edges | 2 |
17+
| Taint edges - number of edges | 3 |
1818
| Taint reach - nodes tainted | 0 |
1919
| Taint reach - per million nodes | 0 |
2020
| Taint sinks - cryptographic operations | 0 |
21-
| Taint sinks - query sinks | 0 |
21+
| Taint sinks - query sinks | 1 |
2222
| Taint sources - active | 0 |
2323
| Taint sources - disabled | 0 |
2424
| Taint sources - sensitive data | 0 |

‎rust/ql/integration-tests/hello-workspace/summary.cargo.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
| Macro calls - resolved | 2 |
1515
| Macro calls - total | 2 |
1616
| Macro calls - unresolved | 0 |
17-
| Taint edges - number of edges | 2 |
17+
| Taint edges - number of edges | 3 |
1818
| Taint reach - nodes tainted | 0 |
1919
| Taint reach - per million nodes | 0 |
2020
| Taint sinks - cryptographic operations | 0 |
21-
| Taint sinks - query sinks | 0 |
21+
| Taint sinks - query sinks | 1 |
2222
| Taint sources - active | 0 |
2323
| Taint sources - disabled | 0 |
2424
| Taint sources - sensitive data | 0 |

‎rust/ql/integration-tests/hello-workspace/summary.rust-project.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
| Macro calls - resolved | 2 |
1515
| Macro calls - total | 2 |
1616
| Macro calls - unresolved | 0 |
17-
| Taint edges - number of edges | 2 |
17+
| Taint edges - number of edges | 3 |
1818
| Taint reach - nodes tainted | 0 |
1919
| Taint reach - per million nodes | 0 |
2020
| Taint sinks - cryptographic operations | 0 |
21-
| Taint sinks - query sinks | 0 |
21+
| Taint sinks - query sinks | 1 |
2222
| Taint sources - active | 0 |
2323
| Taint sources - disabled | 0 |
2424
| Taint sources - sensitive data | 0 |

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ final class ElementContent extends Content, TElementContent {
776776
* NOTE: Unlike `struct`s and `enum`s tuples are structural and not nominal,
777777
* hence we don't store a canonical path for them.
778778
*/
779-
private class TuplePositionContent extends Content, TTuplePositionContent {
779+
final class TuplePositionContent extends Content, TTuplePositionContent {
780780
private int pos;
781781

782782
TuplePositionContent() { this = TTuplePositionContent(pos) }

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/rust-all
4+
extensible: sinkModel
5+
data:
6+
- ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[0]", "log-injection", "manual"] # args
7+
- ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[2]", "log-injection", "manual"] # target
8+
- ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[3]", "log-injection", "manual"] # key value
9+
- ["lang:std", "crate::io::stdio::_print", "Argument[0]", "log-injection", "manual"]
10+
- ["lang:std", "crate::io::stdio::_eprint", "Argument[0]", "log-injection", "manual"]
11+
- ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"]
12+
- ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"]
13+
- ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"]
14+
- ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"]
15+
- ["lang:core", "crate::panicking::panic_fmt", "Argument[0]", "log-injection", "manual"]
16+
- ["lang:core", "crate::panicking::assert_failed", "Argument[3].Variant[crate::option::Option::Some(0)]", "log-injection", "manual"]
17+
- ["lang:core", "<crate::option::Option>::expect", "Argument[0]", "log-injection", "manual"]

‎rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ extensions:
1313
- ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
1414
# String
1515
- ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]
16+
- ["lang:alloc", "<crate::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "manual"]
1617
# Hint
1718
- ["lang:core", "crate::hint::must_use", "Argument[0]", "ReturnValue", "value", "manual"]
1819
# Fmt

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cleartext logging
3+
* of sensitive information vulnerabilities.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.internal.DataFlowImpl
9+
private import codeql.rust.security.SensitiveData
10+
11+
/**
12+
* Provides default sources, sinks and barriers for detecting cleartext logging
13+
* vulnerabilities, as well as extension points for adding your own.
14+
*/
15+
module CleartextLogging {
16+
/**
17+
* A data flow source for cleartext logging vulnerabilities.
18+
*/
19+
abstract class Source extends DataFlow::Node { }
20+
21+
/**
22+
* A data flow sink for cleartext logging vulnerabilities.
23+
*/
24+
abstract class Sink extends DataFlow::Node { }
25+
26+
/**
27+
* A barrier for cleartext logging vulnerabilities.
28+
*/
29+
abstract class Barrier extends DataFlow::Node { }
30+
31+
/**
32+
* Sensitive data, considered as a flow source.
33+
*/
34+
private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
35+
36+
/** A sink for logging from model data. */
37+
private class ModelsAsDataSinks extends Sink {
38+
ModelsAsDataSinks() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
39+
}
40+
}

‎rust/ql/src/queries/security/CWE-312/CleartextLogging.qhelp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Sensitive user data and system information that is logged could be exposed to an attacker when it is
9+
displayed. Also, external processes often store the standard output and standard error streams of
10+
an application, which will include logged sensitive information.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Do not log sensitive data. If it is necessary to log sensitive data, encrypt it before logging.
17+
</p>
18+
</recommendation>
19+
20+
<example>
21+
<p>
22+
The following example code logs user credentials (in this case, their password) in plaintext:
23+
</p>
24+
<sample src="CleartextLoggingBad.rs"/>
25+
<p>
26+
Instead, you should encrypt the credentials, or better still, omit them entirely:
27+
</p>
28+
<sample src="CleartextLoggingGood.rs"/>
29+
</example>
30+
31+
<references>
32+
33+
<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li>
34+
<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
35+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html#data-to-exclude">Logging Cheat Sheet - Data to exclude</a>.</li>
36+
37+
</references>
38+
</qhelp>

‎rust/ql/src/queries/security/CWE-312/CleartextLogging.ql

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @name Cleartext logging of sensitive information
3+
* @description Logging sensitive information in plaintext can
4+
* expose it to an attacker.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id rust/cleartext-logging
10+
* @tags security
11+
* external/cwe/cwe-312
12+
* external/cwe/cwe-359
13+
* external/cwe/cwe-532
14+
*/
15+
16+
import rust
17+
import codeql.rust.security.CleartextLoggingExtensions
18+
import codeql.rust.dataflow.DataFlow
19+
import codeql.rust.dataflow.TaintTracking
20+
import codeql.rust.dataflow.internal.DataFlowImpl
21+
22+
/**
23+
* A taint-tracking configuration for cleartext logging vulnerabilities.
24+
*/
25+
module CleartextLoggingConfig implements DataFlow::ConfigSig {
26+
import CleartextLogging
27+
28+
predicate isSource(DataFlow::Node source) { source instanceof Source }
29+
30+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
31+
32+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
33+
34+
predicate isBarrierIn(DataFlow::Node node) {
35+
// make sources barriers so that we only report the closest instance
36+
isSource(node)
37+
}
38+
39+
predicate isAdditionalFlowStep(Node node1, Node node2) {
40+
// flow from `a` to `&a`
41+
node2.asExpr().getExpr().(RefExpr).getExpr() = node1.asExpr().getExpr()
42+
}
43+
44+
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
45+
// flow out from tuple content at sinks.
46+
isSink(node) and
47+
c.getAReadContent() instanceof TuplePositionContent
48+
}
49+
}
50+
51+
module CleartextLoggingFlow = TaintTracking::Global<CleartextLoggingConfig>;
52+
53+
import CleartextLoggingFlow::PathGraph
54+
55+
from CleartextLoggingFlow::PathNode source, CleartextLoggingFlow::PathNode sink
56+
where CleartextLoggingFlow::flowPath(source, sink)
57+
select sink.getNode(), source, sink, "This operation writes $@ to a log file.", source,
58+
source.getNode().toString()

‎rust/ql/src/queries/security/CWE-312/CleartextLoggingBad.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
let password = "P@ssw0rd";
2+
info!("User password changed to {password}");

0 commit comments

Comments
 (0)