-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Query for cleartext logging of sensitive information #18582
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
Changes from 12 commits
bb3be2f
173cfd5
4297d05
1d2950c
484331c
2bbf493
6444494
78c58aa
59c3ac6
e708167
ccc1243
4214c83
613a165
5570523
d27a71e
951d1fc
44b9a11
f5459d7
814118d
037d496
117db8a
0a3d44c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| extensions: | ||
| - addsTo: | ||
| pack: codeql/rust-all | ||
| extensible: sinkModel | ||
| data: | ||
| - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[0]", "log-injection", "manual"] # args | ||
| - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[2]", "log-injection", "manual"] # target | ||
| - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[3]", "log-injection", "manual"] # key value | ||
| - ["lang:std", "crate::io::stdio::_print", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:std", "crate::io::stdio::_eprint", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:core", "crate::panicking::panic_fmt", "Argument[0]", "log-injection", "manual"] | ||
| - ["lang:core", "crate::panicking::assert_failed", "Argument[3].Variant[crate::option::Option::Some(0)]", "log-injection", "manual"] | ||
| - ["lang:core", "<crate::option::Option>::expect", "Argument[0]", "log-injection", "manual"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
|
|
||
| <overview> | ||
| <p> | ||
| Sensitive user data and system information that is logged could be seen by an attacker when it is | ||
| displayed. Also, external processes often store the standard output and standard error streams of | ||
| an application, which will include logged sensitive information.</p> | ||
| </p> | ||
| </overview> | ||
|
|
||
| <recommendation> | ||
| <p> | ||
| Do not log sensitive data. If it is necessary to log sensitive data, encrypt it before logging. | ||
| </p> | ||
| </recommendation> | ||
|
|
||
| <example> | ||
| <p> | ||
| The following example code logs user credentials (in this case, their password) in plaintext: | ||
| </p> | ||
| <sample src="CleartextLoggingBad.rs"/> | ||
| <p> | ||
| Instead, you should encrypt the credentials, or better still omit them entirely: | ||
geoffw0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| </p> | ||
| <sample src="CleartextLoggingGood.rs"/> | ||
| </example> | ||
|
|
||
| <references> | ||
|
|
||
| <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> | ||
| <li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li> | ||
| <li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html#data-to-exclude">Logging Cheat Sheet - Data to exclude</a>.<li> | ||
|
|
||
| </references> | ||
| </qhelp> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /** | ||
| * @name Cleartext logging of sensitive information | ||
| * @description Logging sensitive information in plaintext can | ||
| * expose it to an attacker. | ||
| * @kind path-problem | ||
| * @problem.severity error | ||
| * @security-severity 7.5 | ||
| * @precision high | ||
| * @id rust/cleartext-logging | ||
| * @tags security | ||
| * external/cwe/cwe-312 | ||
| * external/cwe/cwe-359 | ||
| * external/cwe/cwe-532 | ||
| */ | ||
|
|
||
| import rust | ||
| import codeql.rust.security.CleartextLoggingExtensions | ||
| import codeql.rust.dataflow.DataFlow | ||
| import codeql.rust.dataflow.TaintTracking | ||
| import codeql.rust.dataflow.internal.DataFlowImpl | ||
|
|
||
| /** | ||
| * A taint-tracking configuration for cleartext logging vulnerabilities. | ||
| */ | ||
| module CleartextLoggingConfig implements DataFlow::ConfigSig { | ||
| import CleartextLogging | ||
|
|
||
| predicate isSource(DataFlow::Node source) { source instanceof Source } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { sink instanceof Sink } | ||
|
|
||
| predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier } | ||
|
|
||
| predicate isBarrierIn(DataFlow::Node node) { | ||
| // make sources barriers so that we only report the closest instance | ||
| isSource(node) | ||
| } | ||
|
|
||
| predicate isAdditionalFlowStep(Node node1, Node node2) { | ||
| // flow from `a` to `&a` | ||
| node2.(Node::ExprNode).asExpr().getExpr().(RefExpr).getExpr() = | ||
| node1.(Node::ExprNode).asExpr().getExpr() | ||
| } | ||
|
|
||
| predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { | ||
| // flow out from tuple content at sinks. | ||
| isSink(node) and | ||
| c.getAReadContent() instanceof TuplePositionContent | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't get tuple content to work in the models-as-data sink itself, so this might have to do for now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried the |
||
| } | ||
| } | ||
|
|
||
| module CleartextLoggingFlow = TaintTracking::Global<CleartextLoggingConfig>; | ||
|
|
||
| import CleartextLoggingFlow::PathGraph | ||
|
|
||
| from CleartextLoggingFlow::PathNode source, CleartextLoggingFlow::PathNode sink | ||
| where CleartextLoggingFlow::flowPath(source, sink) | ||
| select sink.getNode(), source, sink, "This operation writes $@ to a log file.", source, | ||
| source.getNode().toString() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| let password = "P@ssw0rd" | ||
geoffw0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| info!("User password changed to {password}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| let password = "P@ssw0rd" | ||
| info!("User password changed") |
Uh oh!
There was an error while loading. Please reload this page.