Skip to content

Commit aad5609

Browse files
atorralbaatorralba
committed
Add Cleartext Loggin query for Swift.
With some caveats: see TODO comments and failing tests.
1 parent 664fdc3 commit aad5609

10 files changed

Lines changed: 328 additions & 0 deletions

File tree

‎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: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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+
// TODO: Remove this. It shouldn't be necessary.
33+
private class EncryptionCleartextLoggingSanitizer extends CleartextLoggingSanitizer {
34+
EncryptionCleartextLoggingSanitizer() { this.asExpr() instanceof EncryptedExpr }
35+
}
36+
37+
/*
38+
* TODO: Add a sanitizer for the OsLogMessage interpolation with .private/.sensitive privacy options,
39+
* or restrict the sinks to require .public interpolation depending on what the default behavior is.
40+
*/
41+
42+
private class LoggingSinks extends SinkModelCsv {
43+
override predicate row(string row) {
44+
row =
45+
[
46+
";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;logging",
47+
";;false;print(_:separator:terminator:);;;Argument[1..2];logging",
48+
";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;logging",
49+
";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];logging",
50+
";;false;NSLog(_:_:);;;Argument[0];logging",
51+
";;false;NSLog(_:_:);;;Argument[1].ArrayElement;logging",
52+
";;false;NSLogv(_:_:);;;Argument[0];logging",
53+
";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;logging",
54+
";;false;vfprintf(_:_:_:);;;Agument[1..2];logging",
55+
";Logger;true;log(_:);;;Argument[0];logging",
56+
";Logger;true;log(level:_:);;;Argument[1];logging",
57+
";Logger;true;trace(_:);;;Argument[1];logging",
58+
";Logger;true;debug(_:);;;Argument[1];logging",
59+
";Logger;true;info(_:);;;Argument[1];logging",
60+
";Logger;true;notice(_:);;;Argument[1];logging",
61+
";Logger;true;warning(_:);;;Argument[1];logging",
62+
";Logger;true;error(_:);;;Argument[1];logging",
63+
";Logger;true;critical(_:);;;Argument[1];logging",
64+
";Logger;true;fault(_:);;;Argument[1];logging",
65+
]
66+
}
67+
}

‎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: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Sensitive information that is logged unencrypted is accessible to an attacker
9+
who gains access to the logs.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
Ensure that sensitive information is always encrypted or obfuscated before being
16+
logged.
17+
</p>
18+
19+
<p>
20+
In general, decrypt sensitive information only at the point where it is
21+
necessary for it to be used in cleartext.
22+
</p>
23+
24+
<p>
25+
Be aware that external processes often store the standard out and
26+
standard error streams of the application, causing logged sensitive
27+
information to be stored.
28+
</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>
33+
The following example code logs user credentials (in this case, their password)
34+
in plain text:
35+
</p>
36+
<sample src="CleartextLoggingBad.swift"/>
37+
<p>
38+
Instead, the credentials should be encrypted, obfuscated, or omitted entirely:
39+
</p>
40+
<sample src="CleartextLoggingGood.swift"/>
41+
</example>
42+
43+
<references>
44+
45+
<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>
46+
<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
47+
<li>OWASP: <a href="https://www.owasp.org/index.php/Password_Plaintext_Storage">Password Plaintext Storage</a>.</li>
48+
49+
</references>
50+
</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 Clear-text logging of sensitive information
3+
* @description Logging sensitive information without encryption or hashing 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‎

Whitespace-only changes.

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

Whitespace-only changes.

‎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+
}

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

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// --- stubs ---
2+
3+
func NSLog(_ format: String, _ args: CVarArg...) {}
4+
func NSLogv(_ format: String, _ args: CVaListPointer) {}
5+
6+
struct OSLogType : RawRepresentable {
7+
static let `default` = OSLogType(rawValue: 0)
8+
let rawValue: UInt8
9+
init(rawValue: UInt8) { self.rawValue = rawValue}
10+
}
11+
12+
struct OSLogStringAlignment {
13+
static var none = OSLogStringAlignment()
14+
}
15+
16+
struct OSLogPrivacy {
17+
enum Mask { case none }
18+
static var auto = OSLogPrivacy()
19+
static var `private` = OSLogPrivacy()
20+
static var sensitive = OSLogPrivacy()
21+
22+
static func auto(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .auto }
23+
static func `private`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .private }
24+
static func `sensitive`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .sensitive }
25+
}
26+
27+
struct OSLogInterpolation : StringInterpolationProtocol {
28+
typealias StringLiteralType = String
29+
init(literalCapacity: Int, interpolationCount: Int) {}
30+
func appendLiteral(_: Self.StringLiteralType) {}
31+
mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
32+
mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
33+
}
34+
35+
struct OSLogMessage : ExpressibleByStringInterpolation {
36+
typealias StringInterpolation = OSLogInterpolation
37+
typealias StringLiteralType = String
38+
typealias ExtendedGraphemeClusterLiteralType = String
39+
typealias UnicodeScalarLiteralType = String
40+
init(stringInterpolation: OSLogInterpolation) {}
41+
init(stringLiteral: String) {}
42+
init(extendedGraphemeClusterLiteral: String) {}
43+
init(unicodeScalarLiteral: String) {}
44+
}
45+
46+
struct Logger {
47+
func log(_ message: OSLogMessage) {}
48+
func log(level: OSLogType, _ message: OSLogMessage) {}
49+
func notice(_: OSLogMessage) {}
50+
func debug(_: OSLogMessage) {}
51+
func trace(_: OSLogMessage) {}
52+
func info(_: OSLogMessage) {}
53+
func error(_: OSLogMessage) {}
54+
func warning(_: OSLogMessage) {}
55+
func fault(_: OSLogMessage) {}
56+
func critical(_: OSLogMessage) {}
57+
58+
}
59+
60+
// --- tests ---
61+
62+
func test1(password: String, passwordHash : String) {
63+
print(password) // $ MISSING: hasCleartextLogging=63
64+
print(password, separator: "") // $ MISSING: $ hasCleartextLogging=64
65+
print("", separator: password) // $ hasCleartextLogging=65
66+
print(password, separator: "", terminator: "") // $ MISSING: hasCleartextLogging=66
67+
print("", separator: password, terminator: "") // $ hasCleartextLogging=67
68+
print("", separator: "", terminator: password) // $ hasCleartextLogging=68
69+
70+
NSLog(password) // $ hasCleartextLogging=70
71+
NSLog("%@", password as! CVarArg) // $ MISSING: hasCleartextLogging=71
72+
NSLog("%@ %@", "" as! CVarArg, password as! CVarArg) // $ MISSING: hasCleartextLogging=72
73+
NSLog("\(password)") // $ hasCleartextLogging=73
74+
NSLogv("%@", getVaList([password as! CVarArg])) // $ MISSING: hasCleartextLogging=74
75+
NSLogv("%@ %@", getVaList(["" as! CVarArg, password as! CVarArg])) // $ MISSING: hasCleartextLogging=75
76+
77+
let log = Logger()
78+
log.log("\(password)") // $ hasCleartextLogging=78
79+
log.log("\(password, privacy: .private)") // Safe
80+
log.log(level: .default, "\(password)") // $ hasCleartextLogging=80
81+
log.trace("\(password)") // $ hasCleartextLogging=81
82+
log.debug("\(password)") // $ hasCleartextLogging=82
83+
log.info("\(password)") // $ hasCleartextLogging=83
84+
log.notice("\(password)") // $ hasCleartextLogging=84
85+
log.warning("\(password)") // $ hasCleartextLogging=85
86+
log.error("\(password)") // $ hasCleartextLogging=86
87+
log.critical("\(password)") // $ hasCleartextLogging=87
88+
log.fault("\(password)") // $ hasCleartextLogging=88
89+
}
90+
/*
91+
class MyClass {
92+
var harmless = "abc"
93+
var password = "123"
94+
}
95+
96+
func test3(x: String) {
97+
// alternative evidence of sensitivity...
98+
99+
UserDefaults.standard.set(x, forKey: "myKey") // $ MISSING: hasCleartextLogging
100+
doSomething(password: x);
101+
UserDefaults.standard.set(x, forKey: "myKey") // $ hasCleartextLogging
102+
103+
let y = getPassword();
104+
UserDefaults.standard.set(y, forKey: "myKey") // $ hasCleartextLogging
105+
106+
let z = MyClass()
107+
UserDefaults.standard.set(z.harmless, forKey: "myKey") // Safe
108+
UserDefaults.standard.set(z.password, forKey: "myKey") // $ hasCleartextLogging
109+
}
110+
111+
func test4(passwd: String) {
112+
// sanitizers...
113+
114+
var x = passwd;
115+
var y = passwd;
116+
var z = passwd;
117+
118+
UserDefaults.standard.set(x, forKey: "myKey") // $ hasCleartextLogging
119+
UserDefaults.standard.set(y, forKey: "myKey") // $ hasCleartextLogging
120+
UserDefaults.standard.set(z, forKey: "myKey") // $ hasCleartextLogging
121+
122+
x = encrypt(x);
123+
hash(data: &y);
124+
z = "";
125+
126+
UserDefaults.standard.set(x, forKey: "myKey") // Safe
127+
UserDefaults.standard.set(y, forKey: "myKey") // Safe
128+
UserDefaults.standard.set(z, forKey: "myKey") // Safe
129+
}
130+
*/

0 commit comments

Comments
 (0)