Skip to content

Commit df72e5e

Browse files
committed
unified: Fix mistranslated Argument tokens
Some argument indices were not translated correctly. In some cases it seems the original index was out of bounds and we set it to the intended argument.
1 parent d92e90f commit df72e5e

2 files changed

Lines changed: 41 additions & 15 deletions

File tree

‎unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ private class Selector extends TSelector {
187187
this = MkSelector(type, true, name, argLabels, _)
188188
)
189189
}
190+
191+
int getPositionalArity() { result = count(int n | this.getArgLabels().splitAt(":", n) = "_") }
192+
193+
string getANamedArgument() { result = this.getArgLabels().splitAt(":") and result != "_" }
190194
}
191195

192196
private predicate isAccessPath(string path) {
@@ -333,3 +337,25 @@ module Public {
333337
predicate isSink(DataFlow::Node node, string kind) { node = getASink(kind, _) }
334338
}
335339
}
340+
341+
private module Debug {
342+
query predicate invalidAccessPath(Selector selector, AccessPathToken ap) {
343+
(
344+
normalizedSourceModel(selector, ap, _, _)
345+
or
346+
normalizedSinkModel(selector, ap, _, _)
347+
) and
348+
ap.getName() = ["Argument", "Parameter"] and
349+
(
350+
exists(int n |
351+
parseInt(ap.getAnArgument()) = n and
352+
not n in [0 .. selector.getPositionalArity() - 1]
353+
)
354+
or
355+
exists(string name |
356+
name = ap.getAnArgument().regexpCapture("(.*):", 1) and
357+
not name = selector.getANamedArgument()
358+
)
359+
)
360+
}
361+
}

‎unified/ql/lib/ext/legacy-swift.model.yml‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ extensions:
9595
- ["", "", false, "NSLog(_:_:)", "", "", "Argument[0,1]", "log-injection", "manual"]
9696
- ["", "", false, "NSLogv(_:_:)", "", "", "Argument[0,1]", "log-injection", "manual"]
9797
- ["", "", false, "os_log(_:)", "", "", "Argument[0]", "log-injection", "manual"]
98-
- ["", "", false, "os_log(_:_:_:dso:log:)", "", "", "Argument[0,4]", "log-injection", "manual"]
99-
- ["", "", false, "os_log(_:_:dso:log:type:)", "", "", "Argument[0,4]", "log-injection", "manual"]
100-
- ["", "", false, "os_log(_:_:log:)", "", "", "Argument[2]", "log-injection", "manual"]
98+
- ["", "", false, "os_log(_:_:_:dso:log:)", "", "", "Argument[0,2]", "log-injection", "manual"]
99+
- ["", "", false, "os_log(_:_:dso:log:type:)", "", "", "Argument[0,1]", "log-injection", "manual"]
100+
- ["", "", false, "os_log(_:_:log:)", "", "", "Argument[1]", "log-injection", "manual"]
101101
- ["", "", false, "print(_:separator:terminator:toStream:)", "", "", "Argument[0,separator:,terminator:]", "log-injection", "manual"]
102102
- ["", "", false, "vfprintf(_:_:_:)", "", "", "Argument[1,2]", "log-injection", "manual"]
103103
- ["", "", false, "sqlite3_bind_blob(_:_:_:_:_:)", "", "", "Argument[2]", "database-store", "manual"]
@@ -272,16 +272,16 @@ extensions:
272272
- ["", "SHA1", true, "calculate(for:)", "", "", "Argument[for:]", "weak-hash-input-SHA1", "manual"]
273273
- ["", "SHA1", true, "process(block:currentHash:)", "", "", "Argument[block:]", "weak-hash-input-SHA1", "manual"]
274274
- ["", "SHA1", true, "update(isLast:withBytes:)", "", "", "Argument[withBytes:]", "weak-hash-input-SHA1", "manual"]
275-
- ["", "Logger", true, "warning(_:)", "", "", "Argument[1]", "log-injection", "manual"]
276-
- ["", "Logger", true, "error(_:)", "", "", "Argument[1]", "log-injection", "manual"]
277-
- ["", "Logger", true, "critical(_:)", "", "", "Argument[1]", "log-injection", "manual"]
278-
- ["", "Logger", true, "debug(_:)", "", "", "Argument[1]", "log-injection", "manual"]
279-
- ["", "Logger", true, "fault(_:)", "", "", "Argument[1]", "log-injection", "manual"]
280-
- ["", "Logger", true, "info(_:)", "", "", "Argument[1]", "log-injection", "manual"]
275+
- ["", "Logger", true, "warning(_:)", "", "", "Argument[0]", "log-injection", "manual"]
276+
- ["", "Logger", true, "error(_:)", "", "", "Argument[0]", "log-injection", "manual"]
277+
- ["", "Logger", true, "critical(_:)", "", "", "Argument[0]", "log-injection", "manual"]
278+
- ["", "Logger", true, "debug(_:)", "", "", "Argument[0]", "log-injection", "manual"]
279+
- ["", "Logger", true, "fault(_:)", "", "", "Argument[0]", "log-injection", "manual"]
280+
- ["", "Logger", true, "info(_:)", "", "", "Argument[0]", "log-injection", "manual"]
281281
- ["", "Logger", true, "log(_:)", "", "", "Argument[0]", "log-injection", "manual"]
282-
- ["", "Logger", true, "log(_:level:)", "", "", "Argument[1]", "log-injection", "manual"]
283-
- ["", "Logger", true, "notice(_:)", "", "", "Argument[1]", "log-injection", "manual"]
284-
- ["", "Logger", true, "trace(_:)", "", "", "Argument[1]", "log-injection", "manual"]
282+
- ["", "Logger", true, "log(_:level:)", "", "", "Argument[0]", "log-injection", "manual"]
283+
- ["", "Logger", true, "notice(_:)", "", "", "Argument[0]", "log-injection", "manual"]
284+
- ["", "Logger", true, "trace(_:)", "", "", "Argument[0]", "log-injection", "manual"]
285285
- ["", "NSException", true, "init(name:reason:userInfo:)", "", "", "Argument[reason:]", "log-injection", "manual"]
286286
- ["", "NSException", true, "raise(_:arguments:format:)", "", "", "Argument[format:,arguments:]", "log-injection", "manual"]
287287
- ["", "SHA256", true, "hash(bufferPointer:)", "", "", "Argument[bufferPointer:]", "weak-password-hash-input-SHA256", "manual"]
@@ -321,11 +321,11 @@ extensions:
321321
- ["", "QueryType", true, "insert(_:)", "", "", "Argument[0]", "database-store", "manual"]
322322
- ["", "QueryType", true, "update(_:)", "", "", "Argument[0]", "database-store", "manual"]
323323
- ["", "QueryType", true, "insert(_:_:)", "", "", "Argument[0,1]", "database-store", "manual"]
324-
- ["", "QueryType", true, "insert(_:or:)", "", "", "Argument[1]", "database-store", "manual"]
324+
- ["", "QueryType", true, "insert(_:or:)", "", "", "Argument[0]", "database-store", "manual"]
325325
- ["", "QueryType", true, "insertMany(_:)", "", "", "Argument[0]", "database-store", "manual"]
326-
- ["", "QueryType", true, "insertMany(_:or:)", "", "", "Argument[1]", "database-store", "manual"]
326+
- ["", "QueryType", true, "insertMany(_:or:)", "", "", "Argument[0]", "database-store", "manual"]
327327
- ["", "QueryType", true, "update(_:_:)", "", "", "Argument[0,1]", "database-store", "manual"]
328-
- ["", "QueryType", true, "update(_:or:)", "", "", "Argument[1]", "database-store", "manual"]
328+
- ["", "QueryType", true, "update(_:or:)", "", "", "Argument[0]", "database-store", "manual"]
329329
- ["", "QueryType", true, "upsert(_:onConflictOf:)", "", "", "Argument[0]", "database-store", "manual"]
330330
- ["", "QueryType", true, "upsert(_:onConflictOf:setValues:)", "", "", "Argument[0]", "database-store", "manual"]
331331
- ["", "QueryType", true, "upsert(_:onConflictOf:setValues:)", "", "", "Argument[setValues:]", "database-store", "manual"]

0 commit comments

Comments
 (0)