diff --git a/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll b/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll index a09dc88aef59..4cb125500a20 100644 --- a/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll +++ b/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll @@ -4,6 +4,7 @@ private import unified private import codeql.dataflow.internal.AccessPathSyntax +private import codeql.unified.internal.typeinference.TypeInference as T extensible predicate legacySourceModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, @@ -29,7 +30,6 @@ private predicate modelContainsMethodName(string name) { } private predicate parsedRawMethodName(string rawName, string name, string argLabels) { - // NOTE: We only support method names at the moment, not property names like `String.count`. modelContainsMethodName(rawName) and exists(string regex | regex = "([^(]+)\\((.*)\\)" and @@ -123,8 +123,34 @@ private predicate callableSelector( ) } +/** Holds if `name` from a model refers to a property, such as `count` in `String.count`. */ +private predicate isRawPropertyName(string name) { + modelContainsMethodName(name) and + not name.matches("%(%") +} + +/** Holds if `cls` is named `type` or, if `subtypes` is true, is a subclass of `type`. */ +bindingset[cls, type] +private predicate classMatchesType(ClassLikeDeclaration cls, string type, boolean subtypes) { + cls.getQualifiedName() = type and subtypes = [true, false] + or + isSubclassOfType(cls, type) and subtypes = true +} + +/** + * Holds if `access` may be an access to the property `name`, and its base has the inferred + * type `cls`. + */ +pragma[nomagic] +private predicate propertyAccessSelector(MemberAccess access, ClassLikeDeclaration cls, string name) { + name = access.getMemberName() and + name = any(Selector s).getPropertyName() and + not access = any(CallExpr call).getCallee() and + cls = T::inferBaseType(access).asClass() +} + private newtype TSelector = - MkSelector(string type, boolean subtypes, string name, string argLabels, string argTypes) { + MkCallSelector(string type, boolean subtypes, string name, string argLabels, string argTypes) { exists(string rawName | legacySourceModel(_, type, subtypes, rawName, argTypes, _, _, _, _) or @@ -134,21 +160,38 @@ private newtype TSelector = | parsedRawMethodName(rawName, name, argLabels) ) + } or + MkPropertySelector(string type, boolean subtypes, string name) { + ( + legacySourceModel(_, type, subtypes, name, _, _, _, _, _) + or + legacySinkModel(_, type, subtypes, name, _, _, _, _, _) + ) and + isRawPropertyName(name) } /** - * A tuple from a model that identifies call sites. + * A tuple from a model that identifies call sites, callables, or property accesses. */ class Selector extends TSelector { - string getTypeString() { this = MkSelector(result, _, _, _, _) } + string getTypeString() { + this = MkCallSelector(result, _, _, _, _) or this = MkPropertySelector(result, _, _) + } + + boolean getSubtypesFlag() { + this = MkCallSelector(_, result, _, _, _) or this = MkPropertySelector(_, result, _) + } - boolean getSubtypesFlag() { this = MkSelector(_, result, _, _, _) } + string getName() { + this = MkCallSelector(_, _, result, _, _) or this = MkPropertySelector(_, _, result) + } - string getName() { this = MkSelector(_, _, result, _, _) } + /** Gets the property name, if this is a property selector. */ + string getPropertyName() { this = MkPropertySelector(_, _, result) } - string getArgLabels() { this = MkSelector(_, _, _, result, _) } + string getArgLabels() { this = MkCallSelector(_, _, _, result, _) } - string getArgTypes() { this = MkSelector(_, _, _, _, result) } + string getArgTypes() { this = MkCallSelector(_, _, _, _, result) } private string ppSubtypes() { if this.getSubtypesFlag() = true then result = "+" else result = "" @@ -158,19 +201,21 @@ class Selector extends TSelector { result = this.getTypeString() + this.ppSubtypes() + "." + this.getName() + "(" + this.getArgLabels() + ")" + this.getArgTypes() + or + result = this.getTypeString() + this.ppSubtypes() + "." + this.getPropertyName() } /** Holds if `name,argLabels` should be used to join with `methodCallSelector`. */ pragma[nomagic] private predicate matchesMethodCallSelector(string name, string argLabels) { - this = MkSelector(_, _, name, argLabels, _) and + this = MkCallSelector(_, _, name, argLabels, _) and name != "init" } /** Holds if `name,argLabels` should be used to join with `constructorCallSelector`. */ pragma[nomagic] private predicate matchesConstructorCallSelector(string name, string argLabels) { - this = MkSelector(name, _, "init", argLabels, _) + this = MkCallSelector(name, _, "init", argLabels, _) } predicate matchesCall(CallExpr call) { @@ -186,7 +231,19 @@ class Selector extends TSelector { predicate matchesCallable(Callable callable) { exists(string type, string name, string argLabels | callableSelector(callable, type, name, argLabels) and - this = MkSelector(type, true, name, argLabels, _) + this = MkCallSelector(type, true, name, argLabels, _) + ) + } + + /** + * Holds if `access` is an access to the property identified by this selector, as determined + * by the inferred type of its base. + */ + predicate matchesPropertyAccess(MemberAccess access) { + exists(ClassLikeDeclaration cls, string type, boolean subtypes, string name | + this = MkPropertySelector(type, subtypes, name) and + propertyAccessSelector(access, cls, name) and + classMatchesType(cls, type, subtypes) ) } @@ -267,27 +324,57 @@ private DataFlow::Node getSourceFromCallable(Callable callable, AccessPathToken result.isReceiverParameter(callable) } +/** + * Gets the source from the property `access` specified by `path`. + */ +bindingset[access, path] +private DataFlow::Node getSourceFromPropertyAccess(MemberAccess access, AccessPath path) { + path = "" and + result.isResultValue(access) +} + +/** + * Gets the sink from the property `access` specified by `path`. + */ +bindingset[access, path] +private DataFlow::Node getSinkFromPropertyAccess(MemberAccess access, AccessPath path) { + path = "PostUpdate" and + ( + // The value being assigned to the property, as in `x.prop = value` + result.isIncomingValue(access) + or + // The property value after being mutated in-place, as in `x.prop.append(value)` + result.isPostUpdate(access) + ) +} + private predicate normalizedSourceModel( Selector selector, AccessPath output, string kind, string provenance ) { - exists( - string type, boolean subtypes, string rawName, string name, string argLabels, string argTypes + exists(string type, boolean subtypes, string rawName, string argTypes | + legacySourceModel(_, type, subtypes, rawName, argTypes, _, output, kind, provenance) | - legacySourceModel(_, type, subtypes, rawName, argTypes, _, output, kind, provenance) and - parsedRawMethodName(rawName, name, argLabels) and - selector = MkSelector(type, subtypes, name, argLabels, argTypes) + exists(string name, string argLabels | + parsedRawMethodName(rawName, name, argLabels) and + selector = MkCallSelector(type, subtypes, name, argLabels, argTypes) + ) + or + selector = MkPropertySelector(type, subtypes, rawName) ) } private predicate normalizedSinkModel( Selector selector, AccessPath output, string kind, string provenance ) { - exists( - string type, boolean subtypes, string rawName, string name, string argLabels, string argTypes + exists(string type, boolean subtypes, string rawName, string argTypes | + legacySinkModel(_, type, subtypes, rawName, argTypes, _, output, kind, provenance) | - legacySinkModel(_, type, subtypes, rawName, argTypes, _, output, kind, provenance) and - parsedRawMethodName(rawName, name, argLabels) and - selector = MkSelector(type, subtypes, name, argLabels, argTypes) + exists(string name, string argLabels | + parsedRawMethodName(rawName, name, argLabels) and + selector = MkCallSelector(type, subtypes, name, argLabels, argTypes) + ) + or + selector = MkPropertySelector(type, subtypes, rawName) ) } @@ -299,7 +386,7 @@ predicate normalizedSummaryModel( | legacySummaryModel(_, type, subtypes, rawName, argTypes, _, input, output, kind, provenance) and parsedRawMethodName(rawName, name, argLabels) and - selector = MkSelector(type, subtypes, name, argLabels, argTypes) + selector = MkCallSelector(type, subtypes, name, argLabels, argTypes) ) } @@ -319,6 +406,11 @@ DataFlow::Node getASource(string kind, string provenance) { selector.matchesCallable(callable) and result = getSourceFromCallable(callable, output) ) + or + exists(MemberAccess access | + selector.matchesPropertyAccess(access) and + result = getSourceFromPropertyAccess(access, output) + ) ) } @@ -338,6 +430,11 @@ DataFlow::Node getASink(string kind, string provenance) { selector.matchesCallable(callable) and result = getSinkFromCallable(callable, input) ) + or + exists(MemberAccess access | + selector.matchesPropertyAccess(access) and + result = getSinkFromPropertyAccess(access, input) + ) ) } diff --git a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll index 2dd7596e29cc..bae2b785c299 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll @@ -331,14 +331,19 @@ module Input3 implements InputSig3 { } } - private Type getImplicitReceiverType(UnqualifiedMemberAccess uma, TypePath path) { - exists(Parameter p, Unified::Callable c | - p.getParameterEx().isImplicitReceiverParameter(c) and - c = uma.getImplicitQualifierVariable().getDeclaringCallable() and - result = p.getType().getTypeAt(path) + additional Type inferTypeFromVariable(LocalVariable v, TypePath path) { + result = inferType(v.getABinding(), path) + or + exists(Parameter param | + implicitParameterDecl(param, v) and + result = param.getType().getTypeAt(path) ) } + private Type getImplicitReceiverType(UnqualifiedMemberAccess uma, TypePath path) { + result = inferTypeFromVariable(uma.getImplicitQualifierVariable(), path) + } + private class UnqualifiedMemberAccessFieldAccess extends FieldAccess, UnqualifiedMemberAccess { override Expr getReceiver() { none() } @@ -862,6 +867,16 @@ predicate inferTypeUnique = M3::inferTypeUnique/2; predicate inferTypeCertain = M3::inferTypeCertain/2; +Type inferVariableType(LocalVariable variable) { + result = Input3::inferTypeFromVariable(variable, _) +} + +Type inferBaseType(MemberAccess access) { + result = inferType(access.(MemberAccessExpr).getBase()) + or + result = inferVariableType(access.(UnqualifiedMemberAccess).getImplicitQualifierVariable()) +} + module Consistency = M3::Consistency; module CachedStage = M3::CachedStage; diff --git a/unified/ql/src/queries/security/CWE-078/CommandInjection.ql b/unified/ql/src/queries/security/CWE-078/CommandInjection.ql index c1268d2bde21..deded6e28220 100644 --- a/unified/ql/src/queries/security/CWE-078/CommandInjection.ql +++ b/unified/ql/src/queries/security/CWE-078/CommandInjection.ql @@ -20,10 +20,7 @@ import unified module CommandInjectionConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { Models::isSource(node, _) } - predicate isSink(DataFlow::Node node) { - // TODO: Support Process property-write sinks from original Swift's CommandInjectionExtensions.qll. - Models::isSink(node, "command-injection") - } + predicate isSink(DataFlow::Node node) { Models::isSink(node, "command-injection") } predicate isBarrier(DataFlow::Node node) { // TODO: Port the numeric-type barrier from original Swift's CommandInjectionExtensions.qll. diff --git a/unified/ql/test/library-tests/mad-builtins/test.expected b/unified/ql/test/library-tests/mad-builtins/test.expected new file mode 100644 index 000000000000..ed4b57f2f172 --- /dev/null +++ b/unified/ql/test/library-tests/mad-builtins/test.expected @@ -0,0 +1,9 @@ +isSink +| test.swift:15:5:15:52 | ... .tlsMinimumSupportedProtocolVersion | tls-protocol-version | +| test.swift:17:5:17:22 | ... .launchPath | command-injection | +| test.swift:18:5:18:21 | ... .arguments | command-injection | +| test.swift:19:5:19:23 | ... .arguments | command-injection | +| test.swift:22:5:22:26 | ... .arguments | command-injection | +isSource +| test.swift:13:9:13:22 | ... .text | local | +missingBuiltinType diff --git a/unified/ql/test/library-tests/mad-builtins/test.qlref b/unified/ql/test/library-tests/mad-builtins/test.qlref new file mode 100644 index 000000000000..f01825b10cf2 --- /dev/null +++ b/unified/ql/test/library-tests/mad-builtins/test.qlref @@ -0,0 +1,2 @@ +query: library-tests/mad/test.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/unified/ql/test/library-tests/mad-builtins/test.swift b/unified/ql/test/library-tests/mad-builtins/test.swift new file mode 100644 index 000000000000..4ba7cea9556e --- /dev/null +++ b/unified/ql/test/library-tests/mad-builtins/test.swift @@ -0,0 +1,23 @@ +// No stubs are declared in this file, so the types below must be resolved via the builtins, +// as is the case when analyzing real code whose dependencies are not extracted. + +class MyProcess: Process {} + +func testPropertiesWithoutStubs( + string: String, + textField: UITextField, + configuration: URLSessionConfiguration, + process: Process, + myProcess: MyProcess +) { + _ = textField.text // $ isSource=local + + configuration.tlsMinimumSupportedProtocolVersion = 0 // $ isSink=tls-protocol-version + + process.launchPath = string // $ isSink=command-injection + process.arguments = [string] // $ isSink=command-injection + myProcess.arguments = [string] // $ isSink=command-injection + + let localProcess = Process() + localProcess.arguments = [string] // $ isSink=command-injection +} diff --git a/unified/ql/test/library-tests/mad/test.expected b/unified/ql/test/library-tests/mad/test.expected index b8648a61054a..6ce3f25f8479 100644 --- a/unified/ql/test/library-tests/mad/test.expected +++ b/unified/ql/test/library-tests/mad/test.expected @@ -6,16 +6,28 @@ isSink | test.swift:21:5:21:16 | [self arg] ... .md5(...) | weak-hash-input-MD5 | | test.swift:23:20:23:25 | string | regex-use | | test.swift:24:20:24:25 | string | regex-use | -| test.swift:71:37:71:49 | encodedOffset | string-length | -| test.swift:72:42:72:54 | encodedOffset | string-length | -| test.swift:76:24:76:36 | encryptionKey | encryption-key | -| test.swift:77:18:77:24 | fileURL | path-injection | -| test.swift:88:24:88:36 | encryptionKey | encryption-key | -| test.swift:89:18:89:24 | fileURL | path-injection | -| test.swift:100:24:100:36 | encryptionKey | encryption-key | -| test.swift:101:18:101:24 | fileURL | path-injection | -| test.swift:107:23:107:34 | seedFilePath | path-injection | -| test.swift:119:29:119:34 | string | path-injection | +| test.swift:55:5:55:45 | ... .tlsMaximumSupportedProtocol | tls-protocol-version | +| test.swift:56:5:56:45 | ... .tlsMinimumSupportedProtocol | tls-protocol-version | +| test.swift:57:5:57:52 | ... .tlsMaximumSupportedProtocolVersion | tls-protocol-version | +| test.swift:58:5:58:52 | ... .tlsMinimumSupportedProtocolVersion | tls-protocol-version | +| test.swift:93:5:93:22 | ... .launchPath | command-injection | +| test.swift:94:5:94:21 | ... .arguments | command-injection | +| test.swift:95:5:95:21 | [post] ... .arguments | command-injection | +| test.swift:96:5:96:23 | ... .arguments | command-injection | +| test.swift:123:37:123:49 | encodedOffset | string-length | +| test.swift:124:42:124:54 | encodedOffset | string-length | +| test.swift:128:24:128:36 | encryptionKey | encryption-key | +| test.swift:129:18:129:24 | fileURL | path-injection | +| test.swift:140:24:140:36 | encryptionKey | encryption-key | +| test.swift:141:18:141:24 | fileURL | path-injection | +| test.swift:152:24:152:36 | encryptionKey | encryption-key | +| test.swift:153:18:153:24 | fileURL | path-injection | +| test.swift:159:23:159:34 | seedFilePath | path-injection | +| test.swift:171:29:171:34 | string | path-injection | +| test.swift:183:9:183:18 | launchPath | command-injection | +| test.swift:184:9:184:17 | arguments | command-injection | +| test.swift:185:9:185:17 | [post] arguments | command-injection | +| test.swift:191:9:191:18 | launchPath | command-injection | isSource | test.swift:4:5:4:27 | String(...) | remote | | test.swift:5:5:5:44 | String(...) | remote | @@ -25,3 +37,9 @@ isSource | test.swift:10:5:12:24 | String(...) | local | | test.swift:13:5:15:28 | String(...) | local | | test.swift:29:29:29:32 | text | local | +| test.swift:52:9:52:20 | ... .count | string-length | +| test.swift:53:9:53:22 | ... .text | local | +| test.swift:89:9:89:24 | ... .text | local | +| test.swift:176:13:176:16 | text | local | +| test.swift:177:13:177:21 | ... .text | local | +missingBuiltinType diff --git a/unified/ql/test/library-tests/mad/test.ql b/unified/ql/test/library-tests/mad/test.ql index 970aaa22e27f..5ee9528d48cd 100644 --- a/unified/ql/test/library-tests/mad/test.ql +++ b/unified/ql/test/library-tests/mad/test.ql @@ -1,5 +1,19 @@ private import unified +private import codeql.unified.internal.Builtins +private import codeql.unified.internal.mad.LegacyMaD as LegacyMaD query predicate isSource = Models::isSource/2; query predicate isSink = Models::isSink/2; + +/** + * Holds if `type` is mentioned in a model, but has no representative in the builtins. + * + * Declarations from dependencies are not extracted, so such types would not be resolved + * by type inference outside of tests. + */ +query predicate missingBuiltinType(string type) { + type = any(LegacyMaD::Selector s).getTypeString() and + type != "" and + not type = any(BuiltinClassLikeDeclaration decl).getQualifiedName() +} diff --git a/unified/ql/test/library-tests/mad/test.swift b/unified/ql/test/library-tests/mad/test.swift index fd1af8f38351..bd0bfb5ed594 100644 --- a/unified/ql/test/library-tests/mad/test.swift +++ b/unified/ql/test/library-tests/mad/test.swift @@ -49,17 +49,69 @@ func testProperties( textField: UITextField, configuration: URLSessionConfiguration ) { - _ = string.count // $ MISSING: isSource=string-length - _ = textField.text // $ MISSING: isSource=local + _ = string.count // $ isSource=string-length + _ = textField.text // $ isSource=local - configuration.tlsMaximumSupportedProtocol = 0 // $ MISSING: isSink=tls-protocol-version - configuration.tlsMinimumSupportedProtocol = 0 // $ MISSING: isSink=tls-protocol-version - configuration.tlsMaximumSupportedProtocolVersion = 0 // $ MISSING: isSink=tls-protocol-version - configuration.tlsMinimumSupportedProtocolVersion = 0 // $ MISSING: isSink=tls-protocol-version + configuration.tlsMaximumSupportedProtocol = 0 // $ isSink=tls-protocol-version + configuration.tlsMinimumSupportedProtocol = 0 // $ isSink=tls-protocol-version + configuration.tlsMaximumSupportedProtocolVersion = 0 // $ isSink=tls-protocol-version + configuration.tlsMinimumSupportedProtocolVersion = 0 // $ isSink=tls-protocol-version + + _ = configuration.tlsMaximumSupportedProtocol // reading a sink property is not a sink + textField.text = "" // writing a source property is not a source +} + +class MyTextField: UITextField {} + +class NotATextField { + var text: String = "" + var arguments: [String] = [] +} + +class Process { + var arguments: [String] = [] + var launchPath: String = "" +} + +class MyProcess: Process {} + +class MyURLSessionConfiguration: URLSessionConfiguration {} + +func testMoreProperties( + myTextField: MyTextField, + notATextField: NotATextField, + unknown: UnknownType, + process: Process, + myProcess: MyProcess, + myConfiguration: MyURLSessionConfiguration, + string: String +) { + _ = myTextField.text // $ isSource=local + _ = notATextField.text // not a source: unrelated type + _ = unknown.text // not a source: unknown type + + process.launchPath = string // $ isSink=command-injection + process.arguments = [string] // $ isSink=command-injection + process.arguments.append(string) // $ isSink=command-injection + myProcess.arguments = [string] // $ isSink=command-injection + notATextField.arguments = [string] // not a sink: unrelated type + + // `tlsMaximumSupportedProtocol` is modeled with subtypes=false + myConfiguration.tlsMaximumSupportedProtocol = 0 } class Realm { - struct Configuration {} + struct Configuration { + var encryptionKey: String = "" + } +} + +func testNestedTypeProperty(configuration: Realm.Configuration, key: String) { + var c1 = Realm.Configuration() + c1.encryptionKey = key // $ MISSING: isSink=encryption-key // nested types are not yet resolved by type inference + + var c2 = configuration + c2.encryptionKey = key // $ MISSING: isSink=encryption-key // nested types are not yet resolved by type inference } func testQualifiedConstructors( @@ -118,3 +170,31 @@ class Connection { func testConnectionLocation(string: String) { Connection.Location.uri(string, parameters: "") // $ isSink=path-injection } + +extension UITextField { + func testImplicitSelfSource() { + _ = text // $ isSource=local + _ = self.text // $ isSource=local + } +} + +extension Process { + func testImplicitSelfSink(string: String) { + launchPath = string // $ isSink=command-injection + arguments = [string] // $ isSink=command-injection + arguments.append(string) // $ isSink=command-injection + } +} + +class MyProcess2: Process { + func testImplicitSelfSinkInSubclass(string: String) { + launchPath = string // $ isSink=command-injection + } +} + +extension NotATextField { + func testImplicitSelfUnrelated(string: String) { + _ = text // not a source: unrelated type + arguments = [string] // not a sink: unrelated type + } +} diff --git a/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.expected b/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.expected index f5936ac854f1..c6df1025804b 100644 --- a/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.expected +++ b/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.expected @@ -1,15 +1,41 @@ #select +| CommandInjection.swift:120:2:120:20 | ... .executableURL | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:120:2:120:20 | ... .executableURL | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | +| CommandInjection.swift:125:2:125:20 | ... .executableURL | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:125:2:125:20 | ... .executableURL | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | +| CommandInjection.swift:126:2:126:20 | ... .executableURL | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:126:2:126:20 | ... .executableURL | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | +| CommandInjection.swift:136:2:136:20 | ... .executableURL | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:136:2:136:20 | ... .executableURL | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | +| CommandInjection.swift:137:2:137:20 | ... .executableURL | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:137:2:137:20 | ... .executableURL | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | +| CommandInjection.swift:146:2:146:16 | [post] ... .arguments | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:146:2:146:16 | [post] ... .arguments | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | | CommandInjection.swift:150:42:150:61 | userControlledString | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:150:42:150:61 | userControlledString | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | | CommandInjection.swift:154:23:154:56 | ... ! | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:154:23:154:56 | ... ! | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | | CommandInjection.swift:158:23:158:56 | ... ! | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:158:23:158:56 | ... ! | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | | CommandInjection.swift:163:41:163:74 | ... ! | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:163:41:163:74 | ... ! | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | String(...) | user-provided value | edges | CommandInjection.swift:105:8:105:31 | ... .some(...) | CommandInjection.swift:105:12:105:31 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:120:36:120:55 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:125:45:125:64 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:126:36:126:55 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:136:45:136:64 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:137:36:137:55 | userControlledString | provenance | | +| CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:146:26:146:45 | userControlledString | provenance | | | CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:150:42:150:61 | userControlledString | provenance | | | CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:154:35:154:54 | userControlledString | provenance | | | CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:158:35:158:54 | userControlledString | provenance | | | CommandInjection.swift:105:12:105:31 | userControlledString | CommandInjection.swift:163:53:163:72 | userControlledString | provenance | | | CommandInjection.swift:105:40:105:94 | String(...) | CommandInjection.swift:105:8:105:31 | ... .some(...) | provenance | | +| CommandInjection.swift:120:24:120:56 | URL(...) | CommandInjection.swift:120:2:120:20 | ... .executableURL | provenance | | +| CommandInjection.swift:120:36:120:55 | userControlledString | CommandInjection.swift:120:2:120:20 | ... .executableURL | provenance | | +| CommandInjection.swift:120:36:120:55 | userControlledString | CommandInjection.swift:120:24:120:56 | URL(...) | provenance | | +| CommandInjection.swift:125:24:125:65 | URL(...) | CommandInjection.swift:125:2:125:20 | ... .executableURL | provenance | | +| CommandInjection.swift:125:45:125:64 | userControlledString | CommandInjection.swift:125:24:125:65 | URL(...) | provenance | | +| CommandInjection.swift:126:24:126:56 | URL(...) | CommandInjection.swift:126:2:126:20 | ... .executableURL | provenance | | +| CommandInjection.swift:126:36:126:55 | userControlledString | CommandInjection.swift:126:2:126:20 | ... .executableURL | provenance | | +| CommandInjection.swift:126:36:126:55 | userControlledString | CommandInjection.swift:126:24:126:56 | URL(...) | provenance | | +| CommandInjection.swift:136:24:136:65 | URL(...) | CommandInjection.swift:136:2:136:20 | ... .executableURL | provenance | | +| CommandInjection.swift:136:45:136:64 | userControlledString | CommandInjection.swift:136:24:136:65 | URL(...) | provenance | | +| CommandInjection.swift:137:24:137:56 | URL(...) | CommandInjection.swift:137:2:137:20 | ... .executableURL | provenance | | +| CommandInjection.swift:137:36:137:55 | userControlledString | CommandInjection.swift:137:2:137:20 | ... .executableURL | provenance | | +| CommandInjection.swift:137:36:137:55 | userControlledString | CommandInjection.swift:137:24:137:56 | URL(...) | provenance | | +| CommandInjection.swift:146:26:146:45 | userControlledString | CommandInjection.swift:146:2:146:16 | [post] ... .arguments | provenance | | | CommandInjection.swift:154:23:154:55 | URL(...) | CommandInjection.swift:154:23:154:56 | ... ! | provenance | | | CommandInjection.swift:154:35:154:54 | userControlledString | CommandInjection.swift:154:23:154:55 | URL(...) | provenance | | | CommandInjection.swift:154:35:154:54 | userControlledString | CommandInjection.swift:154:23:154:56 | ... ! | provenance | | @@ -23,6 +49,23 @@ nodes | CommandInjection.swift:105:8:105:31 | ... .some(...) | semmle.label | ... .some(...) | | CommandInjection.swift:105:12:105:31 | userControlledString | semmle.label | userControlledString | | CommandInjection.swift:105:40:105:94 | String(...) | semmle.label | String(...) | +| CommandInjection.swift:120:2:120:20 | ... .executableURL | semmle.label | ... .executableURL | +| CommandInjection.swift:120:24:120:56 | URL(...) | semmle.label | URL(...) | +| CommandInjection.swift:120:36:120:55 | userControlledString | semmle.label | userControlledString | +| CommandInjection.swift:125:2:125:20 | ... .executableURL | semmle.label | ... .executableURL | +| CommandInjection.swift:125:24:125:65 | URL(...) | semmle.label | URL(...) | +| CommandInjection.swift:125:45:125:64 | userControlledString | semmle.label | userControlledString | +| CommandInjection.swift:126:2:126:20 | ... .executableURL | semmle.label | ... .executableURL | +| CommandInjection.swift:126:24:126:56 | URL(...) | semmle.label | URL(...) | +| CommandInjection.swift:126:36:126:55 | userControlledString | semmle.label | userControlledString | +| CommandInjection.swift:136:2:136:20 | ... .executableURL | semmle.label | ... .executableURL | +| CommandInjection.swift:136:24:136:65 | URL(...) | semmle.label | URL(...) | +| CommandInjection.swift:136:45:136:64 | userControlledString | semmle.label | userControlledString | +| CommandInjection.swift:137:2:137:20 | ... .executableURL | semmle.label | ... .executableURL | +| CommandInjection.swift:137:24:137:56 | URL(...) | semmle.label | URL(...) | +| CommandInjection.swift:137:36:137:55 | userControlledString | semmle.label | userControlledString | +| CommandInjection.swift:146:2:146:16 | [post] ... .arguments | semmle.label | [post] ... .arguments | +| CommandInjection.swift:146:26:146:45 | userControlledString | semmle.label | userControlledString | | CommandInjection.swift:150:42:150:61 | userControlledString | semmle.label | userControlledString | | CommandInjection.swift:154:23:154:55 | URL(...) | semmle.label | URL(...) | | CommandInjection.swift:154:23:154:56 | ... ! | semmle.label | ... ! | diff --git a/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.swift b/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.swift index b822e8a258a8..e22691aad116 100644 --- a/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.swift +++ b/unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.swift @@ -117,13 +117,13 @@ func testCommandInjectionMore(mySafeString: String) { try! task2.run() let task3 = Process() - task3.executableURL = URL(string: userControlledString)! // $ MISSING: Alert + task3.executableURL = URL(string: userControlledString)! // $ Alert task3.arguments = ["abc", userControlledString] // $ MISSING: Alert try! task3.run() let task4 = Process() - task4.executableURL = URL(fileURLWithPath: userControlledString) // $ MISSING: Alert - task4.executableURL = URL(string: userControlledString)! // $ MISSING: Alert + task4.executableURL = URL(fileURLWithPath: userControlledString) // $ Alert + task4.executableURL = URL(string: userControlledString)! // $ Alert task4.arguments = ["abc", "def" + userControlledString] // $ MISSING: Alert try! task4.run() @@ -133,8 +133,8 @@ func testCommandInjectionMore(mySafeString: String) { try! task5?.run() let task6 = MyProcess() - task6.executableURL = URL(fileURLWithPath: userControlledString) // $ MISSING: Alert - task6.executableURL = URL(string: userControlledString)! // $ MISSING: Alert + task6.executableURL = URL(fileURLWithPath: userControlledString) // $ Alert + task6.executableURL = URL(string: userControlledString)! // $ Alert task6.arguments = [userControlledString] // $ MISSING: Alert task6.setArguments([userControlledString]) // BAD (expected inside setArguments; currently missing) task6.harmlessField = userControlledString // GOOD @@ -143,7 +143,7 @@ func testCommandInjectionMore(mySafeString: String) { let task7 = Process() task7.executableURL = URL(fileURLWithPath: mySafeString) // GOOD task7.arguments = ["abc", "def"] - task7.arguments?.append(userControlledString) // $ MISSING: Alert // Originally missing + task7.arguments?.append(userControlledString) // $ Alert // Originally missing try! task7.run() _ = Process.launchedProcess(launchPath: mySafeString, arguments: ["abc", mySafeString]) // GOOD diff --git a/unified/tools/builtins/types.swift b/unified/tools/builtins/types.swift index 5cdac835971a..559f0fc5add1 100644 --- a/unified/tools/builtins/types.swift +++ b/unified/tools/builtins/types.swift @@ -8,6 +8,8 @@ struct String { public init(_ value: T) where T: LosslessStringConvertible { fatalError("Dummy implementation.") } + + struct Index {} } struct Character {} @@ -218,3 +220,242 @@ struct Tuple12 { var _10: T10 var _11: T11 } + +// The types below are referenced by models in `ql/lib/ext/legacy-swift.model.yml`. +// They are included so that type inference can resolve these types (and hence match +// models against property accesses) even though declarations from dependencies are not +// extracted. Only the type hierarchy is represented here, not the members. + +// Swift standard library + +protocol Sequence {} + +protocol IteratorProtocol {} + +protocol Collection: Sequence {} + +protocol BidirectionalCollection: Collection {} + +protocol MutableCollection: Collection {} + +protocol RangeReplaceableCollection: Collection {} + +protocol StringProtocol: BidirectionalCollection {} + +protocol LosslessStringConvertible {} + +protocol TextOutputStream {} + +protocol TextOutputStreamable {} + +protocol RawRepresentable {} + +protocol OptionSet: RawRepresentable {} + +protocol Strideable {} + +protocol Numeric {} + +protocol BinaryInteger: Numeric {} + +protocol SignedInteger: BinaryInteger {} + +protocol UnsignedInteger: BinaryInteger {} + +protocol FixedWidthInteger: BinaryInteger {} + +protocol FloatingPoint: Numeric {} + +protocol BinaryFloatingPoint: FloatingPoint {} + +struct ContiguousArray {} + +struct Slice {} + +struct Regex {} + +// Foundation + +class NSObject {} + +protocol NSCopying {} + +protocol NSMutableCopying {} + +protocol ContiguousBytes {} + +protocol DataProtocol {} + +struct Data: DataProtocol, ContiguousBytes {} + +class NSData: NSObject {} + +class NSMutableData: NSData {} + +class NSString: NSObject {} + +class NSMutableString: NSString {} + +struct NSRange {} + +struct URL { + struct AsyncBytes {} +} + +struct URLResource {} + +class NSURL: NSObject {} + +class NSException: NSObject {} + +class NSKeyedUnarchiver: NSObject {} + +class NSPredicate: NSObject {} + +class NSRegularExpression: NSObject {} + +class NSUserScriptTask: NSObject {} + +class NSUserUnixTask: NSUserScriptTask {} + +class Process: NSObject {} + +class FileManager: NSObject {} + +class Bundle: NSObject {} + +class InputStream: NSObject {} + +class URLSession: NSObject {} + +class URLSessionConfiguration: NSObject {} + +// System, os, Network, AppleArchive + +struct FilePath { + struct Component {} + + struct Root {} +} + +struct Logger {} + +class NWConnection {} + +class ArchiveByteStream {} + +// UIKit, WebKit, JavaScriptCore + +protocol UITextInput {} + +protocol UITextFieldDelegate {} + +protocol UITextViewDelegate {} + +protocol UIApplicationDelegate {} + +protocol UISceneDelegate {} + +class UITextField: NSObject, UITextInput {} + +protocol WKScriptMessageHandler {} + +class WKUserScript: NSObject {} + +class JSContext: NSObject {} + +class JSValue: NSObject {} + +// CryptoKit + +protocol Digest {} + +enum Insecure { + struct MD5 {} + + struct SHA1 {} +} + +struct SHA256 {} + +struct SHA384 {} + +struct SHA512 {} + +// CryptoSwift + +class AES {} + +class Blowfish {} + +class MD5 {} + +class SHA1 {} + +class SHA2 {} + +class SHA3 {} + +// Alamofire + +struct DataResponse {} + +struct DownloadResponse {} + +// GRDB + +protocol DatabaseValueConvertible {} + +protocol FetchableRecord {} + +protocol TableRecord {} + +class Database {} + +class DatabasePool {} + +class DatabaseQueue {} + +class DatabaseSnapshotPool {} + +class SerializedDatabase {} + +class Row {} + +class Statement {} + +class SQLStatementCursor {} + +struct SQL {} + +struct SQLRequest {} + +struct CommonTableExpression {} + +// SQLite.swift + +class Connection { + enum Location {} +} + +struct Blob {} + +struct Expression {} + +protocol ExpressionType {} + +protocol QueryType {} + +// RealmSwift + +struct Realm { + struct Configuration {} +} + +// NIO + +class NIOFileHandle {} + +// Zip + +class Zip {}