Skip to content

Commit ea06ad1

Browse files
geoffw0geoffw0
authored
Merge pull request #11529 from geoffw0/format
Swift: Uncontrolled format string query
2 parents 4c01875 + 9333e80 commit ea06ad1

10 files changed

+340
-0
lines changed

‎swift/ql/lib/codeql/swift/StringFormat.qll

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* Provides classes and predicates for reasoning about string formatting.
3+
*/
4+
5+
import swift
6+
7+
/**
8+
* A function that takes a `printf` style format argument.
9+
*/
10+
abstract class FormattingFunction extends AbstractFunctionDecl {
11+
/**
12+
* Gets the position of the format argument.
13+
*/
14+
abstract int getFormatParameterIndex();
15+
}
16+
17+
/**
18+
* A call to a function that takes a `printf` style format argument.
19+
*/
20+
class FormattingFunctionCall extends CallExpr {
21+
FormattingFunction target;
22+
23+
FormattingFunctionCall() { target = this.getStaticTarget() }
24+
25+
/**
26+
* Gets the format expression used in this call.
27+
*/
28+
Expr getFormat() { result = this.getArgument(target.getFormatParameterIndex()).getExpr() }
29+
}
30+
31+
/**
32+
* An initializer for `String`, `NSString` or `NSMutableString` that takes a
33+
* `printf` style format argument.
34+
*/
35+
class StringInitWithFormat extends FormattingFunction, MethodDecl {
36+
StringInitWithFormat() {
37+
exists(string fName |
38+
this.hasQualifiedName(["String", "NSString", "NSMutableString"], fName) and
39+
fName.matches("init(format:%")
40+
)
41+
}
42+
43+
override int getFormatParameterIndex() { result = 0 }
44+
}
45+
46+
/**
47+
* The `localizedStringWithFormat` method of `String`, `NSString` and `NSMutableString`.
48+
*/
49+
class LocalizedStringWithFormat extends FormattingFunction, MethodDecl {
50+
LocalizedStringWithFormat() {
51+
this.hasQualifiedName(["String", "NSString", "NSMutableString"],
52+
"localizedStringWithFormat(_:_:)")
53+
}
54+
55+
override int getFormatParameterIndex() { result = 0 }
56+
}
57+
58+
/**
59+
* The functions `NSLog` and `NSLogv`.
60+
*/
61+
class NsLog extends FormattingFunction, FreeFunctionDecl {
62+
NsLog() { this.getName() = ["NSLog(_:_:)", "NSLogv(_:_:)"] }
63+
64+
override int getFormatParameterIndex() { result = 0 }
65+
}
66+
67+
/**
68+
* The `NSException.raise` method.
69+
*/
70+
class NsExceptionRaise extends FormattingFunction, MethodDecl {
71+
NsExceptionRaise() { this.hasQualifiedName("NSException", "raise(_:format:arguments:)") }
72+
73+
override int getFormatParameterIndex() { result = 1 }
74+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Provides classes and predicates for reasoning about uncontrolled
3+
* format string vulnerabilities.
4+
*/
5+
6+
import swift
7+
private import codeql.swift.StringFormat
8+
private import codeql.swift.dataflow.DataFlow
9+
private import codeql.swift.dataflow.TaintTracking
10+
private import codeql.swift.dataflow.ExternalFlow
11+
12+
/**
13+
* A dataflow sink for uncontrolled format string vulnerabilities.
14+
*/
15+
abstract class UncontrolledFormatStringSink extends DataFlow::Node { }
16+
17+
/**
18+
* A sanitizer for uncontrolled format string vulnerabilities.
19+
*/
20+
abstract class UncontrolledFormatStringSanitizer extends DataFlow::Node { }
21+
22+
/**
23+
* A unit class for adding additional taint steps.
24+
*/
25+
class UncontrolledFormatStringAdditionalTaintStep extends Unit {
26+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
27+
}
28+
29+
/**
30+
* A default uncontrolled format string sink.
31+
*/
32+
private class DefaultUncontrolledFormatStringSink extends UncontrolledFormatStringSink {
33+
DefaultUncontrolledFormatStringSink() {
34+
// the format argument to a `FormattingFunctionCall`.
35+
this.asExpr() = any(FormattingFunctionCall fc).getFormat()
36+
or
37+
// a sink defined in a Csv model.
38+
sinkNode(this, "uncontrolled-format-string")
39+
}
40+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about uncontrolled
3+
* format string vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.StringFormat
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.TaintTracking
10+
import codeql.swift.dataflow.FlowSources
11+
import codeql.swift.security.UncontrolledFormatStringExtensions
12+
13+
/**
14+
* A taint configuration for tainted data that reaches a format string.
15+
*/
16+
class TaintedFormatConfiguration extends TaintTracking::Configuration {
17+
TaintedFormatConfiguration() { this = "TaintedFormatConfiguration" }
18+
19+
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
20+
21+
override predicate isSink(DataFlow::Node node) { node instanceof UncontrolledFormatStringSink }
22+
23+
override predicate isSanitizer(DataFlow::Node sanitizer) {
24+
sanitizer instanceof UncontrolledFormatStringSanitizer
25+
}
26+
27+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
28+
any(UncontrolledFormatStringAdditionalTaintStep s).step(nodeFrom, nodeTo)
29+
}
30+
}

‎swift/ql/src/queries/Security/CWE-134/UncontrolledFormatString.qhelp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Passing untrusted format strings to functions that use <code>printf</code> style formatting can lead to buffer overflows and data representation problems. An attacker may be able to exploit this weakness to crash the program or obtain sensitive information from its internal state.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>Use a constant string literal for the format string to prevent the possibility of data flow from
13+
an untrusted source. This also helps to prevent errors where the format arguments do not match the format string.</p>
14+
15+
<p>If the format string cannot be constant, ensure that it comes from a secure data source or is compiled into the source code. If you need to include a string value from the user, use an appropriate specifier (such as <code>%@</code>) in the format string and include the user provided value as a format argument.
16+
</p>
17+
18+
</recommendation>
19+
<example>
20+
21+
<p>In this example, the format string includes a user-controlled <code>inputString</code>:</p>
22+
23+
<sample src="UncontrolledFormatStringBad.swift" />
24+
25+
<p>To fix it, make <code>inputString</code> a format argument rather than part of the format string, as in the following code:</p>
26+
27+
<sample src="UncontrolledFormatStringGood.swift" />
28+
29+
</example>
30+
<references>
31+
32+
<li>
33+
OWASP:
34+
<a href="https://owasp.org/www-community/attacks/Format_string_attack">Format string attack</a>.
35+
</li>
36+
37+
</references>
38+
</qhelp>

‎swift/ql/src/queries/Security/CWE-134/UncontrolledFormatString.ql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Uncontrolled format string
3+
* @description Using external input in format strings can lead to exceptions or information leaks.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 9.3
7+
* @precision high
8+
* @id swift/uncontrolled-format-string
9+
* @tags security
10+
* external/cwe/cwe-134
11+
*/
12+
13+
import swift
14+
import codeql.swift.dataflow.DataFlow
15+
import codeql.swift.security.UncontrolledFormatStringQuery
16+
import DataFlow::PathGraph
17+
18+
from TaintedFormatConfiguration config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
19+
where config.hasFlowPath(sourceNode, sinkNode)
20+
select sinkNode.getNode(), sourceNode, sinkNode, "This format string depends on $@.",
21+
sourceNode.getNode(), "this user-provided value"

‎swift/ql/src/queries/Security/CWE-134/UncontrolledFormatStringBad.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
2+
print(String(format: "User input: " + inputString)) // vulnerable

‎swift/ql/src/queries/Security/CWE-134/UncontrolledFormatStringGood.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
2+
print(String(format: "User input: %@", inputString)) // fixed

‎swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
edges
2+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:70:28:70:28 | tainted |
3+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:73:28:73:28 | tainted |
4+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:74:28:74:28 | tainted |
5+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:76:28:76:28 | tainted |
6+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:77:28:77:28 | tainted |
7+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:78:28:78:28 | tainted |
8+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:79:46:79:46 | tainted |
9+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:88:11:88:11 | tainted |
10+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:91:61:91:61 | tainted |
11+
nodes
12+
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | semmle.label | call to String.init(contentsOf:) : |
13+
| UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
14+
| UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |
15+
| UncontrolledFormatString.swift:74:28:74:28 | tainted | semmle.label | tainted |
16+
| UncontrolledFormatString.swift:76:28:76:28 | tainted | semmle.label | tainted |
17+
| UncontrolledFormatString.swift:77:28:77:28 | tainted | semmle.label | tainted |
18+
| UncontrolledFormatString.swift:78:28:78:28 | tainted | semmle.label | tainted |
19+
| UncontrolledFormatString.swift:79:46:79:46 | tainted | semmle.label | tainted |
20+
| UncontrolledFormatString.swift:88:11:88:11 | tainted | semmle.label | tainted |
21+
| UncontrolledFormatString.swift:91:61:91:61 | tainted | semmle.label | tainted |
22+
subpaths
23+
#select
24+
| UncontrolledFormatString.swift:70:28:70:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:70:28:70:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
25+
| UncontrolledFormatString.swift:73:28:73:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:73:28:73:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
26+
| UncontrolledFormatString.swift:74:28:74:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:74:28:74:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
27+
| UncontrolledFormatString.swift:76:28:76:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:76:28:76:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
28+
| UncontrolledFormatString.swift:77:28:77:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:77:28:77:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
29+
| UncontrolledFormatString.swift:78:28:78:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:78:28:78:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
30+
| UncontrolledFormatString.swift:79:46:79:46 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:79:46:79:46 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
31+
| UncontrolledFormatString.swift:88:11:88:11 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:88:11:88:11 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
32+
| UncontrolledFormatString.swift:91:61:91:61 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:91:61:91:61 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |

‎swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/Security/CWE-134/UncontrolledFormatString.ql

‎swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
2+
// --- stubs ---
3+
4+
struct URL
5+
{
6+
init?(string: String) {}
7+
}
8+
9+
struct Locale {
10+
}
11+
12+
extension String : CVarArg {
13+
public var _cVarArgEncoding: [Int] { get { return [] } }
14+
15+
init(contentsOf: URL) throws { self.init() }
16+
init(format: String, _ arguments: CVarArg...) { self.init() }
17+
init(format: String, arguments: [CVarArg]) { self.init() }
18+
init(format: String, locale: Locale?, _ args: CVarArg...) { self.init() }
19+
init(format: String, locale: Locale?, arguments: [CVarArg]) { self.init() }
20+
21+
static func localizedStringWithFormat(_ format: String, _ arguments: CVarArg...) -> String { return "" }
22+
}
23+
24+
class NSObject
25+
{
26+
}
27+
28+
class NSString : NSObject
29+
{
30+
init(string aString: String) {}
31+
init(format: NSString, _ args: CVarArg...) {}
32+
33+
class func localizedStringWithFormat(_ format: NSString, _ args: CVarArg...) {}
34+
}
35+
36+
class NSMutableString : NSString
37+
{
38+
}
39+
40+
struct NSExceptionName {
41+
init(_ rawValue: String) {}
42+
}
43+
44+
class NSException: NSObject
45+
{
46+
class func raise(_ name: NSExceptionName, format: String, arguments argList: CVaListPointer) {}
47+
}
48+
49+
func NSLog(_ format: String, _ args: CVarArg...) {}
50+
51+
func NSLogv(_ format: String, _ args: CVaListPointer) {}
52+
53+
func getVaList(_ args: [CVarArg]) -> CVaListPointer { return (nil as CVaListPointer?)! }
54+
55+
// --- tests ---
56+
57+
func MyLog(_ format: String, _ args: CVarArg...) {
58+
withVaList(args) { arglist in
59+
NSLogv(format, arglist) // BAD [NOT DETECTED]
60+
}
61+
}
62+
63+
func tests() {
64+
let tainted = try! String(contentsOf: URL(string: "http://example.com")!)
65+
66+
let a = String("abc") // GOOD: not a format string
67+
let b = String(tainted) // GOOD: not a format string
68+
69+
let c = String(format: "abc") // GOOD: not tainted
70+
let d = String(format: tainted) // BAD
71+
let e = String(format: "%s", "abc") // GOOD: not tainted
72+
let f = String(format: "%s", tainted) // GOOD: format string itself is not tainted
73+
let g = String(format: tainted, "abc") // BAD
74+
let h = String(format: tainted, tainted) // BAD
75+
76+
let i = String(format: tainted, arguments: []) // BAD
77+
let j = String(format: tainted, locale: nil) // BAD
78+
let k = String(format: tainted, locale: nil, arguments: []) // BAD
79+
let l = String.localizedStringWithFormat(tainted) // BAD
80+
81+
let m = NSString(format: NSString(string: tainted), "abc") // BAD [NOT DETECTED]
82+
let n = NSString.localizedStringWithFormat(NSString(string: tainted)) // BAD [NOT DETECTED]
83+
84+
var o = NSMutableString(format: NSString(string: tainted), "abc") // BAD [NOT DETECTED]
85+
var p = NSMutableString.localizedStringWithFormat(NSString(string: tainted)) // BAD [NOT DETECTED]
86+
87+
NSLog("abc") // GOOD: not tainted
88+
NSLog(tainted) // BAD
89+
MyLog(tainted) // BAD [NOT DETECTED]
90+
91+
NSException.raise(NSExceptionName("exception"), format: tainted, arguments: getVaList([])) // BAD
92+
93+
let taintedVal = Int(tainted)!
94+
let taintedSan = "\(taintedVal)"
95+
let q = String(format: taintedSan) // GOOD: sufficiently sanitized
96+
97+
let taintedVal2 = Int(tainted) ?? 0
98+
let taintedSan2 = String(taintedVal2)
99+
let r = String(format: taintedSan2) // GOOD: sufficiently sanitized
100+
}

0 commit comments

Comments
 (0)