Skip to content

Commit 7e02c05

Browse files
committed
Swift: Address the sprintf case.
1 parent 835967a commit 7e02c05

File tree

2 files changed

+27
-8
lines changed

2 files changed

+27
-8
lines changed

swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,38 @@ private class CleartextLoggingFieldAdditionalFlowStep extends CleartextLoggingAd
9494
}
9595

9696
/**
97-
* A sink that appears to be an imported C `printf` variant.
97+
* A function that appears to be an imported C `printf` variant.
9898
* TODO: merge code with similar cases from the cleartext logging PR.
9999
*/
100+
class PrintfFormat extends FreeFunction {
101+
int formatParamIndex;
102+
string modeChars;
103+
104+
PrintfFormat() {
105+
modeChars = this.getShortName().regexpCapture("(.*)printf.*", 1) and
106+
this.getParam(formatParamIndex).getName() = "format"
107+
}
108+
109+
int getFormatParamIndex() { result = formatParamIndex }
110+
111+
/**
112+
* Holds if this `printf` is a variant of `sprintf`.
113+
*/
114+
predicate isSprintf() { modeChars.charAt(_) = "s" }
115+
}
116+
117+
/**
118+
* A sink that appears to be an imported C `printf` variant.
119+
*/
100120
private class PrintfCleartextLoggingSink extends CleartextLoggingSink {
101121
PrintfCleartextLoggingSink() {
102-
exists(CallExpr ce, FreeFunction f, int formatParamIndex |
103-
f.getShortName().matches("%printf%") and
104-
f.getParam(formatParamIndex).getName() = "format" and
122+
exists(CallExpr ce, PrintfFormat f |
105123
ce.getStaticTarget() = f and
106124
(
107-
this.asExpr() = ce.getArgument(formatParamIndex).getExpr() or
125+
this.asExpr() = ce.getArgument(f.getFormatParamIndex()).getExpr() or
108126
this.asExpr() = ce.getArgument(f.getNumberOfParams() - 1).getExpr()
109-
)
127+
) and
128+
not f.isSprintf()
110129
)
111130
}
112131
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,8 @@ func test6(passwordString: String) {
350350
_ = vprintf("%s is incorrect!", getVaList([passwordString])) // $ hasCleartextLogging=350
351351
_ = vfprintf(nil, "\(passwordString) is incorrect!", getVaList([])) // $ hasCleartextLogging=351
352352
_ = vfprintf(nil, "%s is incorrect!", getVaList([passwordString])) // $ hasCleartextLogging=352
353-
_ = vasprintf_l(nil, nil, "\(passwordString) is incorrect!", getVaList([])) // $ SPURIOUS hasCleartextLogging=353 good (`sprintf` is not logging)
354-
_ = vasprintf_l(nil, nil, "%s is incorrect!", getVaList([passwordString])) // $ SPURIOUS hasCleartextLogging=354 good (`sprintf` is not logging)
353+
_ = vasprintf_l(nil, nil, "\(passwordString) is incorrect!", getVaList([])) // good (`sprintf` is not logging)
354+
_ = vasprintf_l(nil, nil, "%s is incorrect!", getVaList([passwordString])) // good (`sprintf` is not logging)
355355
}
356356

357357
func test7(authKey: String, authKey2: Int, authKey3: Float) {

0 commit comments

Comments
 (0)