Skip to content

Commit bd56191

Browse files
atorralbaatorralba
authored
Merge pull request #11590 from atorralba/atorralba/swift/sensitive-info-logs
Swift: Add Cleartext Logging query
2 parents 8ccc384 + 0017461 commit bd56191

File tree

10 files changed

+392
-0
lines changed

10 files changed

+392
-0
lines changed

‎swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ private module Frameworks {
8989
private import codeql.swift.frameworks.StandardLibrary.UrlSession
9090
private import codeql.swift.frameworks.StandardLibrary.WebView
9191
private import codeql.swift.frameworks.Alamofire.Alamofire
92+
private import codeql.swift.security.CleartextLogging
9293
private import codeql.swift.security.PathInjection
9394
private import codeql.swift.security.PredicateInjection
9495
}

‎swift/ql/lib/codeql/swift/security/CleartextLogging.qll

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/** Provides classes and predicates to reason about cleartext logging of sensitive data vulnerabilities. */
2+
3+
import swift
4+
private import codeql.swift.dataflow.DataFlow
5+
private import codeql.swift.dataflow.ExternalFlow
6+
private import codeql.swift.security.SensitiveExprs
7+
8+
/** A data flow sink for cleartext logging of sensitive data vulnerabilities. */
9+
abstract class CleartextLoggingSink extends DataFlow::Node { }
10+
11+
/** A sanitizer for cleartext logging of sensitive data vulnerabilities. */
12+
abstract class CleartextLoggingSanitizer extends DataFlow::Node { }
13+
14+
/**
15+
* A unit class for adding additional taint steps.
16+
*
17+
* Extend this class to add additional taint steps that should apply to paths related to
18+
* cleartext logging of sensitive data vulnerabilities.
19+
*/
20+
class CleartextLoggingAdditionalTaintStep extends Unit {
21+
/**
22+
* Holds if the step from `n1` to `n2` should be considered a taint
23+
* step for flows related to cleartext logging of sensitive data vulnerabilities.
24+
*/
25+
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
26+
}
27+
28+
private class DefaultCleartextLoggingSink extends CleartextLoggingSink {
29+
DefaultCleartextLoggingSink() { sinkNode(this, "logging") }
30+
}
31+
32+
/**
33+
* A sanitizer for `OSLogMessage`s configured with the appropriate privacy option.
34+
* Numeric and boolean arguments aren't redacted unless the `private` or `sensitive` options are used.
35+
* Arguments of other types are always redacted unless the `public` option is used.
36+
*/
37+
private class OsLogPrivacyCleartextLoggingSanitizer extends CleartextLoggingSanitizer {
38+
OsLogPrivacyCleartextLoggingSanitizer() {
39+
exists(CallExpr c, AutoClosureExpr e |
40+
c.getStaticTarget().getName().matches("appendInterpolation(_:%privacy:%)") and
41+
c.getArgument(0).getExpr() = e and
42+
this.asExpr() = e
43+
|
44+
e.getExpr().getType() instanceof OsLogNonRedactedType and
45+
c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isSafe()
46+
or
47+
not e.getExpr().getType() instanceof OsLogNonRedactedType and
48+
not c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isPublic()
49+
)
50+
}
51+
}
52+
53+
/** A type that isn't redacted by default in an `OSLogMessage`. */
54+
private class OsLogNonRedactedType extends Type {
55+
OsLogNonRedactedType() {
56+
this.getName() = [["", "U"] + "Int" + ["", "8", "16", "32", "64"], "Double", "Float", "Bool"]
57+
}
58+
}
59+
60+
/** A reference to a field of `OsLogPrivacy`. */
61+
private class OsLogPrivacyRef extends MemberRefExpr {
62+
string optionName;
63+
64+
OsLogPrivacyRef() {
65+
exists(FieldDecl f | this.getMember() = f |
66+
f.getEnclosingDecl().(NominalTypeDecl).getName() = "OSLogPrivacy" and
67+
optionName = f.getName()
68+
)
69+
}
70+
71+
/** Holds if this is a safe privacy option (private or sensitive). */
72+
predicate isSafe() { optionName = ["private", "sensitive"] }
73+
74+
/** Holds if this is a public (that is, unsafe) privacy option. */
75+
predicate isPublic() { optionName = "public" }
76+
}
77+
78+
private class LoggingSinks extends SinkModelCsv {
79+
override predicate row(string row) {
80+
row =
81+
[
82+
";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;logging",
83+
";;false;print(_:separator:terminator:);;;Argument[1..2];logging",
84+
";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;logging",
85+
";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];logging",
86+
";;false;NSLog(_:_:);;;Argument[0];logging",
87+
";;false;NSLog(_:_:);;;Argument[1].ArrayElement;logging",
88+
";;false;NSLogv(_:_:);;;Argument[0];logging",
89+
";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;logging",
90+
";;false;vfprintf(_:_:_:);;;Agument[1..2];logging",
91+
";Logger;true;log(_:);;;Argument[0];logging",
92+
";Logger;true;log(level:_:);;;Argument[1];logging",
93+
";Logger;true;trace(_:);;;Argument[1];logging",
94+
";Logger;true;debug(_:);;;Argument[1];logging",
95+
";Logger;true;info(_:);;;Argument[1];logging",
96+
";Logger;true;notice(_:);;;Argument[1];logging",
97+
";Logger;true;warning(_:);;;Argument[1];logging",
98+
";Logger;true;error(_:);;;Argument[1];logging",
99+
";Logger;true;critical(_:);;;Argument[1];logging",
100+
";Logger;true;fault(_:);;;Argument[1];logging",
101+
]
102+
}
103+
}

‎swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about cleartext logging of
3+
* sensitive data vulnerabilities.
4+
*/
5+
6+
import swift
7+
private import codeql.swift.dataflow.DataFlow
8+
private import codeql.swift.dataflow.TaintTracking
9+
private import codeql.swift.security.CleartextLogging
10+
private import codeql.swift.security.SensitiveExprs
11+
12+
/**
13+
* A taint-tracking configuration for cleartext logging of sensitive data vulnerabilities.
14+
*/
15+
class CleartextLoggingConfiguration extends TaintTracking::Configuration {
16+
CleartextLoggingConfiguration() { this = "CleartextLoggingConfiguration" }
17+
18+
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
19+
20+
override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLoggingSink }
21+
22+
override predicate isSanitizer(DataFlow::Node sanitizer) {
23+
sanitizer instanceof CleartextLoggingSanitizer
24+
}
25+
26+
// Disregard paths that contain other paths. This helps with performance.
27+
override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
28+
29+
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
30+
any(CleartextLoggingAdditionalTaintStep s).step(n1, n2)
31+
}
32+
}

‎swift/ql/src/queries/Security/CWE-312/CleartextLogging.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+
6+
<overview>
7+
<p>
8+
Attackers could gain access to sensitive information that is logged unencrypted.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
Always make sure to encrypt or obfuscate sensitive information before you log it.
15+
</p>
16+
17+
<p>
18+
Generally, you should decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
19+
</p>
20+
21+
<p>
22+
Be aware that external processes often store the standard output and
23+
standard error streams of the application. This will include logged sensitive information.
24+
</p>
25+
</recommendation>
26+
27+
<example>
28+
<p>
29+
The following example code logs user credentials (in this case, their password)
30+
in plaintext:
31+
</p>
32+
<sample src="CleartextLoggingBad.swift"/>
33+
<p>
34+
Instead, you should encrypt or obfuscate the credentials, or omit them entirely:
35+
</p>
36+
<sample src="CleartextLoggingGood.swift"/>
37+
</example>
38+
39+
<references>
40+
41+
<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>
42+
<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
43+
<li>OWASP: <a href="https://www.owasp.org/index.php/Password_Plaintext_Storage">Password Plaintext Storage</a>.</li>
44+
45+
</references>
46+
</qhelp>

‎swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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 swift/clear-text-logging
10+
* @tags security
11+
* external/cwe/cwe-312
12+
* external/cwe/cwe-359
13+
* external/cwe/cwe-532
14+
*/
15+
16+
import swift
17+
import codeql.swift.dataflow.DataFlow
18+
import codeql.swift.security.CleartextLoggingQuery
19+
import DataFlow::PathGraph
20+
21+
from DataFlow::PathNode src, DataFlow::PathNode sink
22+
where any(CleartextLoggingConfiguration c).hasFlowPath(src, sink)
23+
select sink.getNode(), src, sink, "This $@ is written to a log file.", src.getNode(),
24+
"potentially sensitive information"

‎swift/ql/src/queries/Security/CWE-312/CleartextLoggingBad.swift

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

‎swift/ql/src/queries/Security/CWE-312/CleartextLoggingGood.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
let password = "P@ssw0rd"
2+
NSLog("User password changed")

‎swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.expected

Whitespace-only changes.

‎swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import swift
2+
import codeql.swift.dataflow.DataFlow
3+
import codeql.swift.security.CleartextLoggingQuery
4+
import TestUtilities.InlineExpectationsTest
5+
6+
class CleartextLogging extends InlineExpectationsTest {
7+
CleartextLogging() { this = "CleartextLogging" }
8+
9+
override string getARelevantTag() { result = "hasCleartextLogging" }
10+
11+
override predicate hasActualResult(Location location, string element, string tag, string value) {
12+
exists(
13+
CleartextLoggingConfiguration config, DataFlow::Node source, DataFlow::Node sink,
14+
Expr sinkExpr
15+
|
16+
config.hasFlow(source, sink) and
17+
sinkExpr = sink.asExpr() and
18+
location = sinkExpr.getLocation() and
19+
element = sinkExpr.toString() and
20+
tag = "hasCleartextLogging" and
21+
value = source.asExpr().getLocation().getStartLine().toString()
22+
)
23+
}
24+
}

0 commit comments

Comments
 (0)