Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit 662f880

Browse files
atorralbaatorralba
authored
Merge pull request #609 from github/atorralba/log-injection-query
Go: Add Log Injection query (CWE-117)
2 parents 5ed4e36 + cc8d9bd commit 662f880

25 files changed

Lines changed: 1527 additions & 22 deletions

File tree

‎change-notes/2021-11-19-log-injection-query.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* A new query "Log entries created from user input" (`go/log-injection`) has been added. The query reports user-provided data reaching calls to logging methods.

‎ql/lib/semmle/go/frameworks/Glog.qll‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,17 @@ import go
1111
*/
1212
module Glog {
1313
private class GlogCall extends LoggerCall::Range, DataFlow::CallNode {
14+
int firstPrintedArg;
15+
1416
GlogCall() {
15-
exists(string pkg, Function f, string fn |
17+
exists(string pkg, Function f, string fn, string level |
1618
pkg = package(["github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog"], "") and
17-
fn.regexpMatch("(Error|Exit|Fatal|Info|Warning)(|f|ln)") and
19+
level = ["Error", "Exit", "Fatal", "Info", "Warning"] and
20+
(
21+
fn = level + ["", "f", "ln"] and firstPrintedArg = 0
22+
or
23+
fn = level + "Depth" and firstPrintedArg = 1
24+
) and
1825
this = f.getACall()
1926
|
2027
f.hasQualifiedName(pkg, fn)
@@ -23,6 +30,8 @@ module Glog {
2330
)
2431
}
2532

26-
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
33+
override DataFlow::Node getAMessageComponent() {
34+
result = this.getArgument(any(int i | i >= firstPrintedArg))
35+
}
2736
}
2837
}

‎ql/lib/semmle/go/frameworks/Logrus.qll‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ module Logrus {
1111

1212
bindingset[result]
1313
private string getALogResultName() {
14-
result.matches(["Debug%", "Error%", "Fatal%", "Info%", "Panic%", "Print%", "Trace%", "Warn%"])
14+
result
15+
.matches([
16+
"Debug%", "Error%", "Fatal%", "Info%", "Log%", "Panic%", "Print%", "Trace%", "Warn%"
17+
])
1518
}
1619

1720
bindingset[result]
@@ -23,7 +26,7 @@ module Logrus {
2326
LogCall() {
2427
exists(string name | name = getALogResultName() or name = getAnEntryUpdatingMethodName() |
2528
this.getTarget().hasQualifiedName(packagePath(), name) or
26-
this.getTarget().(Method).hasQualifiedName(packagePath(), "Entry", name)
29+
this.getTarget().(Method).hasQualifiedName(packagePath(), ["Entry", "Logger"], name)
2730
)
2831
}
2932

‎ql/lib/semmle/go/frameworks/stdlib/Log.qll‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,7 @@ import go
88
module Log {
99
private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
1010
LogCall() {
11-
exists(string fn |
12-
fn.matches("Fatal%")
13-
or
14-
fn.matches("Panic%")
15-
or
16-
fn.matches("Print%")
17-
|
11+
exists(string fn | fn.matches(["Fatal%", "Panic%", "Print%"]) |
1812
this.getTarget().hasQualifiedName("log", fn)
1913
or
2014
this.getTarget().(Method).hasQualifiedName("log", "Logger", fn)

‎ql/lib/semmle/go/security/LogInjection.qll‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about log injection vulnerabilities.
3+
*
4+
* Note: for performance reasons, only import this file if `LogInjection::Configuration` is needed,
5+
* otherwise `LogInjectionCustomizations` should be imported instead.
6+
*/
7+
8+
import go
9+
10+
/**
11+
* Provides a taint-tracking configuration for reasoning about
12+
* log injection vulnerabilities.
13+
*/
14+
module LogInjection {
15+
import LogInjectionCustomizations::LogInjection
16+
17+
/**
18+
* A taint-tracking configuration for reasoning about log injection vulnerabilities.
19+
*/
20+
class Configuration extends TaintTracking::Configuration {
21+
Configuration() { this = "LogInjection" }
22+
23+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
24+
25+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
26+
27+
override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer }
28+
29+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
30+
guard instanceof SanitizerGuard
31+
}
32+
}
33+
}

‎ql/lib/semmle/go/security/LogInjectionCustomizations.qll‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Provides default sources, sinks, and sanitizers for reasoning about
3+
* log injection vulnerabilities, as well as extension points for adding your own.
4+
*/
5+
6+
import go
7+
8+
/**
9+
* Provides extension points for customizing the data-flow tracking configuration for reasoning
10+
* about log injection.
11+
*/
12+
module LogInjection {
13+
/**
14+
* A data flow source for log injection vulnerabilities.
15+
*/
16+
abstract class Source extends DataFlow::Node { }
17+
18+
/**
19+
* A data flow sink for log injection vulnerabilities.
20+
*/
21+
abstract class Sink extends DataFlow::Node { }
22+
23+
/**
24+
* A sanitizer for log injection vulnerabilities.
25+
*/
26+
abstract class Sanitizer extends DataFlow::Node { }
27+
28+
/**
29+
* A sanitizer guard for log injection vulnerabilities.
30+
*/
31+
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
32+
33+
/** A source of untrusted data, considered as a taint source for log injection. */
34+
class UntrustedFlowAsSource extends Source {
35+
UntrustedFlowAsSource() { this instanceof UntrustedFlowSource }
36+
}
37+
38+
/** An argument to a logging mechanism. */
39+
class LoggerSink extends Sink {
40+
LoggerSink() { this = any(LoggerCall log).getAMessageComponent() }
41+
}
42+
}

‎ql/src/Security/CWE-117/LogInjection.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package main
2+
3+
import (
4+
"log"
5+
"net/http"
6+
)
7+
8+
// BAD: A user-provided value is written directly to a log.
9+
func handler(req *http.Request) {
10+
username := req.URL.Query()["username"][0]
11+
log.Printf("user %s logged in.\n", username)
12+
}

‎ql/src/Security/CWE-117/LogInjection.qhelp‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>If unsanitized user input is written to a log entry, a malicious user may
7+
be able to forge new log entries.</p>
8+
9+
<p>Forgery can occur if a user provides some input with characters that are interpreted
10+
when the log output is displayed. If the log is displayed as a plain text file, then new
11+
line characters can be used by a malicious user. If the log is displayed as HTML, then
12+
arbitrary HTML may be included to spoof log entries.</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
User input should be suitably encoded before it is logged.
18+
</p>
19+
<p>
20+
If the log entries are plain text then line breaks should be removed from user input, using
21+
<code>strings.Replace</code> or similar. Care should also be taken that user input is clearly marked
22+
in log entries, and that a malicious user cannot cause confusion in other ways.
23+
</p>
24+
<p>
25+
For log entries that will be displayed in HTML, user input should be HTML encoded using
26+
<code>html.EscapeString</code> or similar before being logged, to prevent forgery and
27+
other forms of HTML injection.
28+
</p>
29+
30+
</recommendation>
31+
32+
<example>
33+
<p>
34+
In the following example, a user name, provided by the user, is logged using a logging framework without any sanitization.
35+
</p>
36+
<sample src="LogInjection.go" />
37+
<p>
38+
In the next example, <code>strings.Replace</code> is used to ensure no line endings are present in the user input.
39+
</p>
40+
<sample src="LogInjectionGood.go" />
41+
</example>
42+
43+
<references>
44+
<li>OWASP: <a href="https://www.owasp.org/index.php/Log_Injection">Log Injection</a>.</li>
45+
</references>
46+
</qhelp>

‎ql/src/Security/CWE-117/LogInjection.ql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Log entries created from user input
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 7.8
8+
* @precision high
9+
* @id go/log-injection
10+
* @tags security
11+
* external/cwe/cwe-117
12+
*/
13+
14+
import go
15+
import semmle.go.security.LogInjection
16+
import DataFlow::PathGraph
17+
18+
from LogInjection::Configuration c, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where c.hasFlowPath(source, sink)
20+
select sink, source, sink, "This log write receives unsanitized user input from $@.",
21+
source.getNode(), "here"

‎ql/src/Security/CWE-117/LogInjectionGood.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package main
2+
3+
import (
4+
"log"
5+
"net/http"
6+
"strings"
7+
)
8+
9+
// GOOD: The user-provided value is escaped before being written to the log.
10+
func handlerGood(req *http.Request) {
11+
username := req.URL.Query()["username"][0]
12+
escapedUsername := strings.Replace(username, "\n", "", -1)
13+
escapedUsername = strings.Replace(escapedUsername, "\r", "", -1)
14+
log.Printf("user %s logged in.\n", escapedUsername)
15+
}

0 commit comments

Comments
 (0)