hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

Swift: Fix imprecise sinks.

Browse Source
This commit is contained in:
Geoffrey White
2023-05-19 17:24:50 +01:00
parent 19080333b9
commit 2028b5ef95
3 changed files with 73 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
swift/ql/lib/codeql/swift/security/StringLengthConflationExtensions.qll
View File

@@ -180,14 +180,6 @@ private class StringLengthConflationSinks extends SinkModelCsv {
override predicate row(string row) {
row =
[
";Sequence;true;dropFirst(_:);;;Argument[0];string-length",
";Sequence;true;dropLast(_:);;;Argument[0];string-length",
";Sequence;true;prefix(_:);;;Argument[0];string-length",
";Sequence;true;suffix(_:);;;Argument[0];string-length",
";Collection;true;formIndex(_:offsetBy:);;;Argument[0..1];string-length",
";Collection;true;formIndex(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
";Collection;true;removeFirst(_:);;;Argument[0];string-length",
";RangeReplaceableCollection;true;removeLast(_:);;;Argument[0];string-length",
";String;true;index(_:offsetBy:);;;Argument[0..1];string-length",
";String;true;index(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
";String.Index;true;init(encodedOffset:);;;Argument[0];string-length",
@@ -203,3 +195,52 @@ private class StringLengthConflationSinks extends SinkModelCsv {
]
}
}
/**
* An extra sink that don't fit into the CSV scheme (because we care about the actual
* type the method is being called on, not just the type it's declared on).
*/
private class ExtraStringLengthConflationSink extends StringLengthConflationSink {
StringType stringType;
ExtraStringLengthConflationSink() {
exists(CallExpr call, string typeName |
(
// `String`
typeName = "String" and
stringType = TString()
or
// `String.utf8`
typeName = "String.UTF8View" and
stringType = TStringUtf8()
or
// `String.utf16`
typeName = "String.UTF16View" and
stringType = TStringUtf16()
or
// `String.unicodeScalars`
typeName = "String.UnicodeScalarView" and
stringType = TStringUnicodeScalars()
) and
// sink is a length or offset argument to [type].[method]
(
call.getQualifier().getType().(NominalType).getName() = typeName or
call.getQualifier().getType().(InOutType).getObjectType().(NominalType).getName() = typeName
) and
(
call.getStaticTarget().getName() =
[
"dropFirst(_:)", "dropLast(_:)", "prefix(_:)", "suffix(_:)", "removeFirst(_:)",
"removeLast(_:)"
] and
this.asExpr() = call.getArgument(0).getExpr()
or
call.getStaticTarget().getName() =
["formIndex(_:offsetBy:)", "formIndex(_:offsetBy:limitBy:)"] and
this.asExpr() = call.getArgument([0, 1]).getExpr()
)
)
}
override StringType getCorrectStringType() { result = stringType }
}

36
swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected
View File

@@ -23,13 +23,13 @@ edges
| StringLengthConflation.swift:170:29:170:46 | .count | StringLengthConflation.swift:170:29:170:54 | ... .-(_:_:) ... |
| StringLengthConflation.swift:171:29:171:32 | .length | StringLengthConflation.swift:171:29:171:41 | ... .-(_:_:) ... |
| StringLengthConflation.swift:172:29:172:33 | .length | StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... |
| StringLengthConflation.swift:173:35:173:37 | .count | StringLengthConflation.swift:173:35:173:45 | ... .-(_:_:) ... |
| StringLengthConflation.swift:174:35:174:42 | .count | StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... |
| StringLengthConflation.swift:175:35:175:43 | .count | StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... |
| StringLengthConflation.swift:176:35:176:52 | .count | StringLengthConflation.swift:176:35:176:60 | ... .-(_:_:) ... |
| StringLengthConflation.swift:177:35:177:38 | .length | StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... |
| StringLengthConflation.swift:178:35:178:39 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... |
| StringLengthConflation.swift:180:37:180:44 | .count | StringLengthConflation.swift:180:37:180:52 | ... .-(_:_:) ... |
| StringLengthConflation.swift:182:37:182:45 | .count | StringLengthConflation.swift:182:37:182:53 | ... .-(_:_:) ... |
| StringLengthConflation.swift:179:37:179:39 | .count | StringLengthConflation.swift:179:37:179:47 | ... .-(_:_:) ... |
| StringLengthConflation.swift:181:37:181:39 | .count | StringLengthConflation.swift:181:37:181:47 | ... .-(_:_:) ... |
| file://:0:0:0:0 | .length | StringLengthConflation.swift:53:43:53:46 | .length |
| file://:0:0:0:0 | .length | StringLengthConflation.swift:60:47:60:50 | .length |
| file://:0:0:0:0 | .length | StringLengthConflation.swift:66:33:66:36 | .length |
@@ -102,20 +102,20 @@ nodes
| StringLengthConflation.swift:171:29:171:41 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:172:29:172:33 | .length | semmle.label | .length |
| StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:173:35:173:37 | .count | semmle.label | .count |
| StringLengthConflation.swift:173:35:173:45 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:174:35:174:42 | .count | semmle.label | .count |
| StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:175:35:175:43 | .count | semmle.label | .count |
| StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:176:35:176:52 | .count | semmle.label | .count |
| StringLengthConflation.swift:176:35:176:60 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:177:35:177:38 | .length | semmle.label | .length |
| StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:178:35:178:39 | .length | semmle.label | .length |
| StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:180:37:180:44 | .count | semmle.label | .count |
| StringLengthConflation.swift:180:37:180:52 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:182:37:182:45 | .count | semmle.label | .count |
| StringLengthConflation.swift:182:37:182:53 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:179:37:179:39 | .count | semmle.label | .count |
| StringLengthConflation.swift:179:37:179:47 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| StringLengthConflation.swift:181:37:181:39 | .count | semmle.label | .count |
| StringLengthConflation.swift:181:37:181:47 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
| file://:0:0:0:0 | .length | semmle.label | .length |
subpaths
#select
@@ -164,12 +164,12 @@ subpaths
| StringLengthConflation.swift:171:29:171:41 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:171:29:171:41 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... | StringLengthConflation.swift:172:29:172:33 | .length | StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:172:29:172:42 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... | StringLengthConflation.swift:174:35:174:42 | .count | StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... | This String.utf8 length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... | StringLengthConflation.swift:175:35:175:43 | .count | StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... | This String.utf16 length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:176:35:176:60 | ... .-(_:_:) ... | StringLengthConflation.swift:176:35:176:52 | .count | StringLengthConflation.swift:176:35:176:60 | ... .-(_:_:) ... | This String.unicodeScalars length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | StringLengthConflation.swift:177:35:177:38 | .length | StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | StringLengthConflation.swift:178:35:178:39 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:180:37:180:52 | ... .-(_:_:) ... | StringLengthConflation.swift:180:37:180:44 | .count | StringLengthConflation.swift:180:37:180:52 | ... .-(_:_:) ... | This String.utf8 length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:182:37:182:53 | ... .-(_:_:) ... | StringLengthConflation.swift:182:37:182:45 | .count | StringLengthConflation.swift:182:37:182:53 | ... .-(_:_:) ... | This String.utf16 length is used in a String, but it may not be equivalent. |
| StringLengthConflation.swift:173:35:173:45 | ... .-(_:_:) ... | StringLengthConflation.swift:173:35:173:37 | .count | StringLengthConflation.swift:173:35:173:45 | ... .-(_:_:) ... | This String length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... | StringLengthConflation.swift:174:35:174:42 | .count | StringLengthConflation.swift:174:35:174:50 | ... .-(_:_:) ... | This String.utf8 length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... | StringLengthConflation.swift:175:35:175:43 | .count | StringLengthConflation.swift:175:35:175:51 | ... .-(_:_:) ... | This String.utf16 length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | StringLengthConflation.swift:177:35:177:38 | .length | StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | This NSString length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:177:35:177:47 | ... .-(_:_:) ... | This NSString length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | StringLengthConflation.swift:178:35:178:39 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | This NSString length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | file://:0:0:0:0 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | This NSString length is used in a String.unicodeScalars, but it may not be equivalent. |
| StringLengthConflation.swift:179:37:179:47 | ... .-(_:_:) ... | StringLengthConflation.swift:179:37:179:39 | .count | StringLengthConflation.swift:179:37:179:47 | ... .-(_:_:) ... | This String length is used in a String.utf8, but it may not be equivalent. |
| StringLengthConflation.swift:181:37:181:47 | ... .-(_:_:) ... | StringLengthConflation.swift:181:37:181:39 | .count | StringLengthConflation.swift:181:37:181:47 | ... .-(_:_:) ... | This String length is used in a String.utf16, but it may not be equivalent. |

12
swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift
View File

@@ -170,16 +170,16 @@ func test(s: String) {
let _ = String(s.prefix(s.unicodeScalars.count - 10)) // BAD
let _ = String(s.prefix(ns.length - 10)) // BAD
let _ = String(s.prefix(nms.length - 10)) // BAD
let _ = String(scalars.prefix(s.count - 10)) // BAD [NOT DETECTED]
let _ = String(scalars.prefix(s.count - 10)) // BAD
let _ = String(scalars.prefix(s.utf8.count - 10)) // BAD
let _ = String(scalars.prefix(s.utf16.count - 10)) // BAD
let _ = String(scalars.prefix(s.unicodeScalars.count - 10)) // GOOD [FALSE POSITIVE]
let _ = String(scalars.prefix(s.unicodeScalars.count - 10)) // GOOD
let _ = String(scalars.prefix(ns.length - 10)) // BAD
let _ = String(scalars.prefix(nms.length - 10)) // BAD
let _ = String(s.utf8.dropFirst(s.count - 10)) // BAD [NOT DETECTED]
let _ = String(s.utf8.dropFirst(s.utf8.count - 10)) // GOOD [FALSE POSITIVE]
let _ = String(s.utf16.dropLast(s.count - 10)) // BAD [NOT DETECTED]
let _ = String(s.utf16.dropLast(s.utf16.count - 10)) // GOOD [FALSE POSITIVE]
let _ = String(s.utf8.dropFirst(s.count - 10)) // BAD
let _ = String(s.utf8.dropFirst(s.utf8.count - 10)) // GOOD
let _ = String(s.utf16.dropLast(s.count - 10)) // BAD
let _ = String(s.utf16.dropLast(s.utf16.count - 10)) // GOOD
}
// `begin :thumbsup: end`, with thumbs up emoji and skin tone modifier