Merge pull request #11529 from geoffw0/format

Swift: Uncontrolled format string query
This commit is contained in:
Geoffrey White
2023-01-17 16:16:10 +00:00
committed by GitHub
10 changed files with 340 additions and 0 deletions

View File

@@ -0,0 +1,74 @@
/**
* Provides classes and predicates for reasoning about string formatting.
*/
import swift
/**
* A function that takes a `printf` style format argument.
*/
abstract class FormattingFunction extends AbstractFunctionDecl {
/**
* Gets the position of the format argument.
*/
abstract int getFormatParameterIndex();
}
/**
* A call to a function that takes a `printf` style format argument.
*/
class FormattingFunctionCall extends CallExpr {
FormattingFunction target;
FormattingFunctionCall() { target = this.getStaticTarget() }
/**
* Gets the format expression used in this call.
*/
Expr getFormat() { result = this.getArgument(target.getFormatParameterIndex()).getExpr() }
}
/**
* An initializer for `String`, `NSString` or `NSMutableString` that takes a
* `printf` style format argument.
*/
class StringInitWithFormat extends FormattingFunction, MethodDecl {
StringInitWithFormat() {
exists(string fName |
this.hasQualifiedName(["String", "NSString", "NSMutableString"], fName) and
fName.matches("init(format:%")
)
}
override int getFormatParameterIndex() { result = 0 }
}
/**
* The `localizedStringWithFormat` method of `String`, `NSString` and `NSMutableString`.
*/
class LocalizedStringWithFormat extends FormattingFunction, MethodDecl {
LocalizedStringWithFormat() {
this.hasQualifiedName(["String", "NSString", "NSMutableString"],
"localizedStringWithFormat(_:_:)")
}
override int getFormatParameterIndex() { result = 0 }
}
/**
* The functions `NSLog` and `NSLogv`.
*/
class NsLog extends FormattingFunction, FreeFunctionDecl {
NsLog() { this.getName() = ["NSLog(_:_:)", "NSLogv(_:_:)"] }
override int getFormatParameterIndex() { result = 0 }
}
/**
* The `NSException.raise` method.
*/
class NsExceptionRaise extends FormattingFunction, MethodDecl {
NsExceptionRaise() { this.hasQualifiedName("NSException", "raise(_:format:arguments:)") }
override int getFormatParameterIndex() { result = 1 }
}

View File

@@ -0,0 +1,40 @@
/**
* Provides classes and predicates for reasoning about uncontrolled
* format string vulnerabilities.
*/
import swift
private import codeql.swift.StringFormat
private import codeql.swift.dataflow.DataFlow
private import codeql.swift.dataflow.TaintTracking
private import codeql.swift.dataflow.ExternalFlow
/**
* A dataflow sink for uncontrolled format string vulnerabilities.
*/
abstract class UncontrolledFormatStringSink extends DataFlow::Node { }
/**
* A sanitizer for uncontrolled format string vulnerabilities.
*/
abstract class UncontrolledFormatStringSanitizer extends DataFlow::Node { }
/**
* A unit class for adding additional taint steps.
*/
class UncontrolledFormatStringAdditionalTaintStep extends Unit {
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
}
/**
* A default uncontrolled format string sink.
*/
private class DefaultUncontrolledFormatStringSink extends UncontrolledFormatStringSink {
DefaultUncontrolledFormatStringSink() {
// the format argument to a `FormattingFunctionCall`.
this.asExpr() = any(FormattingFunctionCall fc).getFormat()
or
// a sink defined in a Csv model.
sinkNode(this, "uncontrolled-format-string")
}
}

View File

@@ -0,0 +1,30 @@
/**
* Provides a taint-tracking configuration for reasoning about uncontrolled
* format string vulnerabilities.
*/
import swift
import codeql.swift.StringFormat
import codeql.swift.dataflow.DataFlow
import codeql.swift.dataflow.TaintTracking
import codeql.swift.dataflow.FlowSources
import codeql.swift.security.UncontrolledFormatStringExtensions
/**
* A taint configuration for tainted data that reaches a format string.
*/
class TaintedFormatConfiguration extends TaintTracking::Configuration {
TaintedFormatConfiguration() { this = "TaintedFormatConfiguration" }
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
override predicate isSink(DataFlow::Node node) { node instanceof UncontrolledFormatStringSink }
override predicate isSanitizer(DataFlow::Node sanitizer) {
sanitizer instanceof UncontrolledFormatStringSanitizer
}
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
any(UncontrolledFormatStringAdditionalTaintStep s).step(nodeFrom, nodeTo)
}
}

View File

@@ -0,0 +1,38 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
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>
</overview>
<recommendation>
<p>Use a constant string literal for the format string to prevent the possibility of data flow from
an untrusted source. This also helps to prevent errors where the format arguments do not match the format string.</p>
<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.
</p>
</recommendation>
<example>
<p>In this example, the format string includes a user-controlled <code>inputString</code>:</p>
<sample src="UncontrolledFormatStringBad.swift" />
<p>To fix it, make <code>inputString</code> a format argument rather than part of the format string, as in the following code:</p>
<sample src="UncontrolledFormatStringGood.swift" />
</example>
<references>
<li>
OWASP:
<a href="https://owasp.org/www-community/attacks/Format_string_attack">Format string attack</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,21 @@
/**
* @name Uncontrolled format string
* @description Using external input in format strings can lead to exceptions or information leaks.
* @kind path-problem
* @problem.severity error
* @security-severity 9.3
* @precision high
* @id swift/uncontrolled-format-string
* @tags security
* external/cwe/cwe-134
*/
import swift
import codeql.swift.dataflow.DataFlow
import codeql.swift.security.UncontrolledFormatStringQuery
import DataFlow::PathGraph
from TaintedFormatConfiguration config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
where config.hasFlowPath(sourceNode, sinkNode)
select sinkNode.getNode(), sourceNode, sinkNode, "This format string depends on $@.",
sourceNode.getNode(), "this user-provided value"

View File

@@ -0,0 +1,2 @@
print(String(format: "User input: " + inputString)) // vulnerable

View File

@@ -0,0 +1,2 @@
print(String(format: "User input: %@", inputString)) // fixed

View File

@@ -0,0 +1,32 @@
edges
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:70:28:70:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:73:28:73:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:74:28:74:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:76:28:76:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:77:28:77:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:78:28:78:28 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:79:46:79:46 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:88:11:88:11 | tainted |
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | UncontrolledFormatString.swift:91:61:91:61 | tainted |
nodes
| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) : | semmle.label | call to String.init(contentsOf:) : |
| UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:74:28:74:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:76:28:76:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:77:28:77:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:78:28:78:28 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:79:46:79:46 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:88:11:88:11 | tainted | semmle.label | tainted |
| UncontrolledFormatString.swift:91:61:91:61 | tainted | semmle.label | tainted |
subpaths
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |

View File

@@ -0,0 +1 @@
queries/Security/CWE-134/UncontrolledFormatString.ql

View File

@@ -0,0 +1,100 @@
// --- stubs ---
struct URL
{
init?(string: String) {}
}
struct Locale {
}
extension String : CVarArg {
public var _cVarArgEncoding: [Int] { get { return [] } }
init(contentsOf: URL) throws { self.init() }
init(format: String, _ arguments: CVarArg...) { self.init() }
init(format: String, arguments: [CVarArg]) { self.init() }
init(format: String, locale: Locale?, _ args: CVarArg...) { self.init() }
init(format: String, locale: Locale?, arguments: [CVarArg]) { self.init() }
static func localizedStringWithFormat(_ format: String, _ arguments: CVarArg...) -> String { return "" }
}
class NSObject
{
}
class NSString : NSObject
{
init(string aString: String) {}
init(format: NSString, _ args: CVarArg...) {}
class func localizedStringWithFormat(_ format: NSString, _ args: CVarArg...) {}
}
class NSMutableString : NSString
{
}
struct NSExceptionName {
init(_ rawValue: String) {}
}
class NSException: NSObject
{
class func raise(_ name: NSExceptionName, format: String, arguments argList: CVaListPointer) {}
}
func NSLog(_ format: String, _ args: CVarArg...) {}
func NSLogv(_ format: String, _ args: CVaListPointer) {}
func getVaList(_ args: [CVarArg]) -> CVaListPointer { return (nil as CVaListPointer?)! }
// --- tests ---
func MyLog(_ format: String, _ args: CVarArg...) {
withVaList(args) { arglist in
NSLogv(format, arglist) // BAD [NOT DETECTED]
}
}
func tests() {
let tainted = try! String(contentsOf: URL(string: "http://example.com")!)
let a = String("abc") // GOOD: not a format string
let b = String(tainted) // GOOD: not a format string
let c = String(format: "abc") // GOOD: not tainted
let d = String(format: tainted) // BAD
let e = String(format: "%s", "abc") // GOOD: not tainted
let f = String(format: "%s", tainted) // GOOD: format string itself is not tainted
let g = String(format: tainted, "abc") // BAD
let h = String(format: tainted, tainted) // BAD
let i = String(format: tainted, arguments: []) // BAD
let j = String(format: tainted, locale: nil) // BAD
let k = String(format: tainted, locale: nil, arguments: []) // BAD
let l = String.localizedStringWithFormat(tainted) // BAD
let m = NSString(format: NSString(string: tainted), "abc") // BAD [NOT DETECTED]
let n = NSString.localizedStringWithFormat(NSString(string: tainted)) // BAD [NOT DETECTED]
var o = NSMutableString(format: NSString(string: tainted), "abc") // BAD [NOT DETECTED]
var p = NSMutableString.localizedStringWithFormat(NSString(string: tainted)) // BAD [NOT DETECTED]
NSLog("abc") // GOOD: not tainted
NSLog(tainted) // BAD
MyLog(tainted) // BAD [NOT DETECTED]
NSException.raise(NSExceptionName("exception"), format: tainted, arguments: getVaList([])) // BAD
let taintedVal = Int(tainted)!
let taintedSan = "\(taintedVal)"
let q = String(format: taintedSan) // GOOD: sufficiently sanitized
let taintedVal2 = Int(tainted) ?? 0
let taintedSan2 = String(taintedVal2)
let r = String(format: taintedSan2) // GOOD: sufficiently sanitized
}