From cf7f355fc4d96ebc4b9449431edeb3f194c11be1 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:08:26 +0100 Subject: [PATCH 1/2] Swift: Additional test cases. --- .../CWE-135/StringLengthConflation.expected | 21 +++++++++++++++++++ .../CWE-135/StringLengthConflation.swift | 9 ++++++++ 2 files changed, 30 insertions(+) diff --git a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected index 909c6233ba5..60868298616 100644 --- a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected +++ b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected @@ -2,6 +2,8 @@ edges | StringLengthConflation2.swift:35:36:35:38 | .count | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | | StringLengthConflation2.swift:37:34:37:36 | .count | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | | StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | +| StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | +| StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | | StringLengthConflation.swift:60:47:60:50 | .length | StringLengthConflation.swift:60:47:60:59 | ... ./(_:_:) ... | | StringLengthConflation.swift:66:33:66:36 | .length | StringLengthConflation.swift:66:33:66:45 | ... ./(_:_:) ... | | StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:36:30:36:37 | len | @@ -30,6 +32,9 @@ edges | StringLengthConflation.swift:178:35:178:39 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | | 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 | ... .-(_:_:) ... | +| StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:36:30:36:37 | len | +| StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:36:30:36:37 | len | +| StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:36:30:36:37 | len | | 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 | @@ -49,6 +54,10 @@ nodes | StringLengthConflation2.swift:37:34:37:36 | .count | semmle.label | .count | | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... | | StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | +| StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | +| StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | +| StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | +| StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | | StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | | StringLengthConflation.swift:53:43:53:46 | .length | semmle.label | .length | | StringLengthConflation.swift:54:43:54:50 | .count | semmle.label | .count | @@ -116,12 +125,21 @@ nodes | 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 | ... .-(_:_:) ... | +| StringLengthConflation.swift:190:28:190:28 | .count | semmle.label | .count | +| StringLengthConflation.swift:190:28:190:28 | .count | semmle.label | .count | +| StringLengthConflation.swift:191:28:191:33 | .count | semmle.label | .count | +| StringLengthConflation.swift:191:28:191:33 | .count | semmle.label | .count | +| StringLengthConflation.swift:193:28:193:43 | .count | semmle.label | .count | +| StringLengthConflation.swift:193:28:193:43 | .count | semmle.label | .count | | file://:0:0:0:0 | .length | semmle.label | .length | subpaths #select | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | StringLengthConflation2.swift:35:36:35:38 | .count | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. | | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | StringLengthConflation2.swift:37:34:37:36 | .count | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. | | StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String length is used in an NSString, but it may not be equivalent. | +| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String length is used in an NSString, but it may not be equivalent. | +| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String.UTF8View length is used in an NSString, but it may not be equivalent. | +| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String.UnicodeScalarView length is used in an NSString, but it may not be equivalent. | | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | This NSString length is used in a String, but it may not be equivalent. | | StringLengthConflation.swift:53:43:53:46 | .length | file://:0:0:0:0 | .length | StringLengthConflation.swift:53:43:53:46 | .length | This NSString length is used in a String, but it may not be equivalent. | | StringLengthConflation.swift:54:43:54:50 | .count | StringLengthConflation.swift:54:43:54:50 | .count | StringLengthConflation.swift:54:43:54:50 | .count | This String.UTF8View length is used in a String, but it may not be equivalent. | @@ -173,3 +191,6 @@ subpaths | 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.UnicodeScalarView, 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.UTF8View, 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.UTF16View, but it may not be equivalent. | +| StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:190:28:190:28 | .count | This String length is used in an NSString, but it may not be equivalent. | +| StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:191:28:191:33 | .count | This String.UTF8View length is used in an NSString, but it may not be equivalent. | +| StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:193:28:193:43 | .count | This String.UnicodeScalarView length is used in an NSString, but it may not be equivalent. | diff --git a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift index c707b81fe35..5b9137ad04d 100644 --- a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift +++ b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift @@ -184,3 +184,12 @@ func test(s: String) { // `begin :thumbsup: end`, with thumbs up emoji and skin tone modifier test(s: "begin \u{0001F44D}\u{0001F3FF} end") + +extension String { + func newStringMethod() { + _ = NSMakeRange(0, count) // BAD + _ = NSMakeRange(0, utf8.count) // BAD + _ = NSMakeRange(0, utf16.count) // GOOD (`String.UTF16View` and `NSString` lengths are equivalent) + _ = NSMakeRange(0, unicodeScalars.count) // BAD + } +} From 9f683b8630d35e494e42c76c32df5694fdcce1fd Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:38:58 +0100 Subject: [PATCH 2/2] Swift: Remove duplicate results. --- .../security/StringLengthConflationQuery.qll | 5 +++++ .../CWE-135/StringLengthConflation.expected | 21 ------------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/swift/ql/lib/codeql/swift/security/StringLengthConflationQuery.qll b/swift/ql/lib/codeql/swift/security/StringLengthConflationQuery.qll index af3ae1f7cc1..8e608776a20 100644 --- a/swift/ql/lib/codeql/swift/security/StringLengthConflationQuery.qll +++ b/swift/ql/lib/codeql/swift/security/StringLengthConflationQuery.qll @@ -31,6 +31,11 @@ module StringLengthConflationConfig implements DataFlow::StateConfigSig { predicate isBarrier(DataFlow::Node barrier) { barrier instanceof StringLengthConflationBarrier } + predicate isBarrierOut(DataFlow::Node node) { + // make sinks barriers so that we only report the closest instance + isSink(node, _) + } + predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { any(StringLengthConflationAdditionalFlowStep s).step(nodeFrom, nodeTo) } diff --git a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected index 60868298616..0fa5e727fcb 100644 --- a/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected +++ b/swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected @@ -1,12 +1,8 @@ edges | StringLengthConflation2.swift:35:36:35:38 | .count | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | | StringLengthConflation2.swift:37:34:37:36 | .count | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | -| StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | -| StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | -| StringLengthConflation.swift:36:30:36:37 | len | StringLengthConflation.swift:36:93:36:93 | len | | StringLengthConflation.swift:60:47:60:50 | .length | StringLengthConflation.swift:60:47:60:59 | ... ./(_:_:) ... | | StringLengthConflation.swift:66:33:66:36 | .length | StringLengthConflation.swift:66:33:66:45 | ... ./(_:_:) ... | -| StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:36:30:36:37 | len | | StringLengthConflation.swift:96:28:96:31 | .length | StringLengthConflation.swift:96:28:96:40 | ... .-(_:_:) ... | | StringLengthConflation.swift:100:27:100:30 | .length | StringLengthConflation.swift:100:27:100:39 | ... .-(_:_:) ... | | StringLengthConflation.swift:104:25:104:28 | .length | StringLengthConflation.swift:104:25:104:37 | ... .-(_:_:) ... | @@ -32,9 +28,6 @@ edges | StringLengthConflation.swift:178:35:178:39 | .length | StringLengthConflation.swift:178:35:178:48 | ... .-(_:_:) ... | | 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 | ... .-(_:_:) ... | -| StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:36:30:36:37 | len | -| StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:36:30:36:37 | len | -| StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:36:30:36:37 | len | | 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 | @@ -53,12 +46,6 @@ nodes | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... | | StringLengthConflation2.swift:37:34:37:36 | .count | semmle.label | .count | | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... | -| StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | -| StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | -| StringLengthConflation.swift:36:30:36:37 | len | semmle.label | len | -| StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | -| StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | -| StringLengthConflation.swift:36:93:36:93 | len | semmle.label | len | | StringLengthConflation.swift:53:43:53:46 | .length | semmle.label | .length | | StringLengthConflation.swift:54:43:54:50 | .count | semmle.label | .count | | StringLengthConflation.swift:55:43:55:51 | .count | semmle.label | .count | @@ -68,7 +55,6 @@ nodes | StringLengthConflation.swift:66:33:66:36 | .length | semmle.label | .length | | StringLengthConflation.swift:66:33:66:45 | ... ./(_:_:) ... | semmle.label | ... ./(_:_:) ... | | StringLengthConflation.swift:72:33:72:35 | .count | semmle.label | .count | -| StringLengthConflation.swift:72:33:72:35 | .count | semmle.label | .count | | StringLengthConflation.swift:78:47:78:49 | .count | semmle.label | .count | | StringLengthConflation.swift:79:47:79:54 | .count | semmle.label | .count | | StringLengthConflation.swift:81:47:81:64 | .count | semmle.label | .count | @@ -126,20 +112,13 @@ nodes | StringLengthConflation.swift:181:37:181:39 | .count | semmle.label | .count | | StringLengthConflation.swift:181:37:181:47 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... | | StringLengthConflation.swift:190:28:190:28 | .count | semmle.label | .count | -| StringLengthConflation.swift:190:28:190:28 | .count | semmle.label | .count | | StringLengthConflation.swift:191:28:191:33 | .count | semmle.label | .count | -| StringLengthConflation.swift:191:28:191:33 | .count | semmle.label | .count | -| StringLengthConflation.swift:193:28:193:43 | .count | semmle.label | .count | | StringLengthConflation.swift:193:28:193:43 | .count | semmle.label | .count | | file://:0:0:0:0 | .length | semmle.label | .length | subpaths #select | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | StringLengthConflation2.swift:35:36:35:38 | .count | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. | | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | StringLengthConflation2.swift:37:34:37:36 | .count | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. | -| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String length is used in an NSString, but it may not be equivalent. | -| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:190:28:190:28 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String length is used in an NSString, but it may not be equivalent. | -| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:191:28:191:33 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String.UTF8View length is used in an NSString, but it may not be equivalent. | -| StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:193:28:193:43 | .count | StringLengthConflation.swift:36:93:36:93 | len | This String.UnicodeScalarView length is used in an NSString, but it may not be equivalent. | | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | This NSString length is used in a String, but it may not be equivalent. | | StringLengthConflation.swift:53:43:53:46 | .length | file://:0:0:0:0 | .length | StringLengthConflation.swift:53:43:53:46 | .length | This NSString length is used in a String, but it may not be equivalent. | | StringLengthConflation.swift:54:43:54:50 | .count | StringLengthConflation.swift:54:43:54:50 | .count | StringLengthConflation.swift:54:43:54:50 | .count | This String.UTF8View length is used in a String, but it may not be equivalent. |