Skip to content

Commit 49c4c34

Browse files
geoffw0geoffw0
authored
Merge pull request #20221 from github/copilot/fix-20220
Rust: Implement a new query for Log Injection
2 parents 374c547 + e84135a commit 49c4c34

File tree

15 files changed

+2066
-0
lines changed

15 files changed

+2066
-0
lines changed

‎rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
14+
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
1415
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1516
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1617
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql

‎rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
14+
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
1415
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1516
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1617
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* Provides classes and predicates for reasoning about log injection
3+
* 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+
private import codeql.util.Unit
11+
12+
/**
13+
* Provides default sources, sinks and barriers for detecting log injection
14+
* vulnerabilities, as well as extension points for adding your own.
15+
*/
16+
module LogInjection {
17+
/**
18+
* A data flow source for log injection vulnerabilities.
19+
*/
20+
abstract class Source extends DataFlow::Node { }
21+
22+
/**
23+
* A data flow sink for log injection vulnerabilities.
24+
*/
25+
abstract class Sink extends QuerySink::Range {
26+
override string getSinkType() { result = "LogInjection" }
27+
}
28+
29+
/**
30+
* A barrier for log injection vulnerabilities.
31+
*/
32+
abstract class Barrier extends DataFlow::Node { }
33+
34+
/**
35+
* An active threat-model source, considered as a flow source.
36+
*/
37+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
38+
39+
/**
40+
* A sink for log-injection from model data.
41+
*/
42+
private class ModelsAsDataSink extends Sink {
43+
ModelsAsDataSink() { sinkNode(this, "log-injection") }
44+
}
45+
}

‎rust/ql/src/change-notes/2025-08-18-log-injection.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `rust/log-injection`, for detecting cases where log entries could be forged by a malicious user.

‎rust/ql/src/queries/security/CWE-117/LogInjection.qhelp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
8+
<p>If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.</p>
9+
10+
<p>Forgery can occur if a user provides some input with characters that are interpreted
11+
when the log output is displayed. If the log is displayed as a plain text file, then new
12+
line characters can be used by a malicious user. If the log is displayed as HTML, then
13+
arbitrary HTML may be included to spoof log entries.</p>
14+
</overview>
15+
16+
<recommendation>
17+
<p>
18+
User input should be suitably sanitized before it is logged.
19+
</p>
20+
<p>
21+
If the log entries are in plain text, then line breaks should be removed from user input using
22+
<code>String::replace</code> or similar. Care should also be taken that user input is clearly marked
23+
in log entries.
24+
</p>
25+
<p>
26+
For log entries that will be displayed in HTML, user input should be HTML-encoded before being logged, to prevent forgery and
27+
other forms of HTML injection.
28+
</p>
29+
30+
</recommendation>
31+
32+
<example>
33+
<p>In the first example, a username, provided by the user via command line arguments, is logged using the <code>log</code> crate.
34+
If a malicious user provides <code>Guest\n[INFO] User: Admin\n</code> as a username parameter,
35+
the log entry will be split into multiple lines, where the second line will appear as <code>[INFO] User: Admin</code>,
36+
potentially forging a legitimate admin login entry.
37+
</p>
38+
<sample src="LogInjectionBad.rs" />
39+
40+
<p>In the second example, <code>String::replace</code> is used to ensure no line endings are present in the user input before logging.</p>
41+
<sample src="LogInjectionGood.rs" />
42+
</example>
43+
44+
<references>
45+
<li>OWASP: <a href="https://owasp.org/www-community/attacks/Log_Injection">Log Injection</a>.</li>
46+
</references>
47+
</qhelp>

‎rust/ql/src/queries/security/CWE-117/LogInjection.ql

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* @name Log injection
3+
* @description Building log entries from user-controlled sources is vulnerable to
4+
* insertion of forged log entries by a malicious user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 2.6
8+
* @precision medium
9+
* @id rust/log-injection
10+
* @tags security
11+
* external/cwe/cwe-117
12+
*/
13+
14+
import rust
15+
import codeql.rust.dataflow.DataFlow
16+
import codeql.rust.dataflow.TaintTracking
17+
import codeql.rust.security.LogInjectionExtensions
18+
19+
/**
20+
* A taint configuration for tainted data that reaches a log injection sink.
21+
*/
22+
module LogInjectionConfig implements DataFlow::ConfigSig {
23+
import LogInjection
24+
25+
predicate isSource(DataFlow::Node node) { node instanceof Source }
26+
27+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
28+
29+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
30+
31+
predicate observeDiffInformedIncrementalMode() { any() }
32+
}
33+
34+
module LogInjectionFlow = TaintTracking::Global<LogInjectionConfig>;
35+
36+
import LogInjectionFlow::PathGraph
37+
38+
from LogInjectionFlow::PathNode sourceNode, LogInjectionFlow::PathNode sinkNode
39+
where LogInjectionFlow::flowPath(sourceNode, sinkNode)
40+
select sinkNode.getNode(), sourceNode, sinkNode, "Log entry depends on a $@.", sourceNode.getNode(),
41+
"user-provided value"

‎rust/ql/src/queries/security/CWE-117/LogInjectionBad.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
use std::env;
2+
use log::info;
3+
4+
fn main() {
5+
env_logger::init();
6+
7+
// Get username from command line arguments
8+
let args: Vec<String> = env::args().collect();
9+
let username = args.get(1).unwrap_or(&String::from("Guest")).clone();
10+
11+
// BAD: log message constructed with unsanitized user input
12+
info!("User login attempt: {}", username);
13+
}

‎rust/ql/src/queries/security/CWE-117/LogInjectionGood.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
use std::env;
2+
use log::info;
3+
4+
fn sanitize_for_logging(input: &str) -> String {
5+
// Remove newlines and carriage returns to prevent log injection
6+
input.replace('\n', "").replace('\r', "")
7+
}
8+
9+
fn main() {
10+
env_logger::init();
11+
12+
// Get username from command line arguments
13+
let args: Vec<String> = env::args().collect();
14+
let username = args.get(1).unwrap_or(&String::from("Guest")).clone();
15+
16+
// GOOD: log message constructed with sanitized user input
17+
let sanitized_username = sanitize_for_logging(username.as_str());
18+
info!("User login attempt: {}", sanitized_username);
19+
}

‎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.LogInjectionExtensions
2526
private import codeql.rust.security.SqlInjectionExtensions
2627
private import codeql.rust.security.TaintedPathExtensions
2728
private import codeql.rust.security.UncontrolledAllocationSizeExtensions

‎rust/ql/test/query-tests/security/CWE-117/CONSISTENCY/PathResolutionConsistency.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
multipleCallTargets
2+
| main.rs:9:43:9:63 | ...::from(...) |
3+
| main.rs:44:19:44:32 | username.len() |

0 commit comments

Comments
 (0)