diff --git a/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll b/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll index b0b48abf6f4c..9c4b9ee89f17 100644 --- a/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll +++ b/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll @@ -129,7 +129,7 @@ module TaintReach { } module TaintReachConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { Models::isSource(node, _) } + predicate isSource(DataFlow::Node node) { Models::isSource(node, ["local", "remote"]) } predicate isSink(DataFlow::Node node) { node instanceof Candidate } } diff --git a/unified/ql/lib/codeql/unified/internal/ExprPositions.qll b/unified/ql/lib/codeql/unified/internal/ExprPositions.qll index 7de4b5e195d8..e3e8efda8044 100644 --- a/unified/ql/lib/codeql/unified/internal/ExprPositions.qll +++ b/unified/ql/lib/codeql/unified/internal/ExprPositions.qll @@ -43,7 +43,7 @@ predicate isInSyntacticTypeContext(Expr expr) { } private predicate isInTypeContext0(Expr expr) { - exists(NameBinding::getTypeFromRef(expr)) + NameBinding::isTypeRef(expr) or isInTypeContext0(expr.getEnclosingExpr()) } diff --git a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll index a559dab61029..e0edd388534e 100644 --- a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll @@ -7,6 +7,7 @@ module; private import unified private import codeql.unified.internal.LocalNameBinding private import codeql.unified.internal.NameBindingPlugin +private import codeql.unified.internal.mad.LegacyMaD as MaD cached private newtype TNameBindingNode = @@ -21,7 +22,8 @@ private newtype TNameBindingNode = } or TModuleScope(ModuleScopeRepr repr) or TFolderScope(Folder folder) or - TModuleRoot() + TModuleRoot() or + TTypeSelector(MaD::TypeSelector type) /** * A node in a graph, in which name-binding rules are represented as edges between nodes. @@ -61,6 +63,8 @@ class NameBindingNode extends TNameBindingNode { /** Holds if this represents the root namespace in which all named modules are members. */ predicate isModuleRoot() { this = TModuleRoot() } + predicate isTypeSelector(MaD::TypeSelector type) { this = TTypeSelector(type) } + /** * Gets an AST node wrapped by this name-binding node, if such a node exists. * @@ -104,6 +108,10 @@ class NameBindingNode extends TNameBindingNode { exists(Folder folder | this.isFolderScope(folder) and result = "FolderScope(" + folder + ")") or this.isModuleRoot() and result = "ModuleRoot" + or + exists(MaD::TypeSelector type | + this.isTypeSelector(type) and result = "TypeSelector(" + type + ")" + ) } Location getLocation() { @@ -224,6 +232,11 @@ predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) { ) or FolderHeuristic::storeStep(node1, name, node2) + or + exists(MaD::TypeSelector type | + node1.isTypeSelector(type.append(name)) and + node2.isTypeSelector(type) + ) } predicate valueStep(NameBindingNode node1, NameBindingNode node2) { @@ -290,6 +303,11 @@ predicate valueStep(NameBindingNode node1, NameBindingNode node2) { node2.isLocalName(self) and self.hasNameAndScope(any(NameBindingPlugin p).getStaticSelfName(cls), cls) ) + or + exists(TopLevel top | + node1.isTypeSelector("") and + node2.isLocalNamespace(top.getBody()) + ) } private predicate isImportPrefix(Expr e) { @@ -529,6 +547,8 @@ private module TrackNameBindingInput implements TrackInputSig { node.isIdentifier(decl) and not isTrivialNameAlias(decl) ) + or + node.isTypeSelector(_) } } @@ -542,6 +562,14 @@ private NameBindingNode trackNameBinding(NameBinding decl) { ) } +/** Gets a name-binding node that may refer to the given type from a model. */ +NameBindingNode trackTypeSelector(MaD::TypeSelector type) { + exists(NameBindingNode start | + start.isTypeSelector(type) and + result = TrackNameBinding::track(start) + ) +} + /** Holds if `node` should be included in the debug view. */ private signature predicate relevantNodeSig(AstNode node); @@ -842,6 +870,14 @@ Identifier getTypeFromRef(AstNode access) { ) } +/** Holds if `access` is a reference to a type. */ +predicate isTypeRef(AstNode access) { + exists(getTypeFromRef(access)) + or + not exists(getStaticBindingTargetFromRef(access)) and + trackTypeSelector(_) = getNodeFromRef(access) +} + /** * Gets the name of the implicit receiver parameter in scope at `callable` (possibly declared by an outer callable). * diff --git a/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll b/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll index 09da505b7d0b..610b20258f7e 100644 --- a/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll +++ b/unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll @@ -4,6 +4,9 @@ private import unified private import codeql.dataflow.internal.AccessPathSyntax +private import codeql.unified.internal.typeinference.TypeInference as T +private import codeql.unified.internal.typeinference.Type as Types +private import codeql.unified.internal.StaticNameBinding as N extensible predicate legacySourceModel( string namespace, string type, boolean subtypes, string name, string signature, string ext, @@ -29,7 +32,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 @@ -76,11 +78,11 @@ private predicate methodCallSelector(CallExpr call, string name, string argLabel } pragma[nomagic] -private predicate constructorCallSelector(CallExpr call, string name, string argLabels) { +private predicate constructorCallSelector(CallExpr call, TypeSelector type, string argLabels) { ( if call.getCallee().(MemberAccessExpr).getMemberName() = "init" - then name = getQualifiedNameFromExpr(call.getCallee().(MemberAccessExpr).getBase()) - else name = getQualifiedNameFromExpr(call.getCallee()) + then type.getAStaticRef() = call.getCallee().(MemberAccessExpr).getBase() + else type.getAStaticRef() = call.getCallee() ) and argLabels = getArgLabelsFromCall(call) } @@ -123,8 +125,26 @@ 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 `access` may be an access to the property `name`, and its base has the inferred + * type `baseType`. + */ +pragma[nomagic] +private predicate propertyAccessSelector(MemberAccess access, T::Type baseType, string name) { + name = access.getMemberName() and + name = any(Selector s).getPropertyName() and + not access = any(CallExpr call).getCallee() and + baseType = T::inferBaseType(access) +} + 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 +154,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 = MkSelector(_, result, _, _, _) } + boolean getSubtypesFlag() { + this = MkCallSelector(_, result, _, _, _) or this = MkPropertySelector(_, result, _) + } - string getName() { this = MkSelector(_, _, result, _, _) } + string getName() { + this = MkCallSelector(_, _, result, _, _) or this = MkPropertySelector(_, _, result) + } - string getArgLabels() { this = MkSelector(_, _, _, result, _) } + /** Gets the property name, if this is a property selector. */ + string getPropertyName() { this = MkPropertySelector(_, _, result) } - string getArgTypes() { this = MkSelector(_, _, _, _, result) } + string getArgLabels() { this = MkCallSelector(_, _, _, result, _) } + + string getArgTypes() { this = MkCallSelector(_, _, _, _, result) } private string ppSubtypes() { if this.getSubtypesFlag() = true then result = "+" else result = "" @@ -158,19 +195,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 +225,18 @@ 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(TypeSelector type, boolean subtypes, string name | + this = MkPropertySelector(type, subtypes, name) and + propertyAccessSelector(access, type.getAnInstanceType([false, subtypes]), name) ) } @@ -195,6 +245,81 @@ class Selector extends TSelector { string getANamedArgument() { result = this.getArgLabels().splitAt(":") and result != "_" } } +/** Gets the `n`th `.`-separated component in the given `type` string. */ +pragma[nomagic] +private string getTypeComponent(string type, int n) { + type = any(Selector s).getTypeString() and + result = type.splitAt(".", n) +} + +/** Holds if the `n`th prefix of `type` is `prefix`, which itself consists of `qualifier` followed by a single `component`. */ +private predicate splitTypeString( + string type, int n, string qualifier, string component, string prefix +) { + component = getTypeComponent(type, n) and + ( + n = 0 and qualifier = "" and prefix = component + or + splitTypeString(type, n - 1, _, _, qualifier) and + prefix = qualifier + "." + component + ) +} + +pragma[nomagic] +private string typeSelectorCons(string qualifier, string component) { + splitTypeString(_, _, qualifier, component, result) +} + +/** A type, or prefix thereof, mentioned in a model. */ +class TypeSelector extends string { + TypeSelector() { splitTypeString(_, _, _, _, this) } + + TypeSelector append(string component) { result = typeSelectorCons(this, component) } + + TypeSelector getQualifier() { this = result.append(_) } + + string getHead() { this = typeSelectorCons(_, result) } + + /** Gets a static reference to this type. */ + AstNode getAStaticRef() { + N::trackTypeSelector(this) = N::getNodeFromRef(result) + or + // When the class has a declaration in the database, such as a built-in or stub class in a test case, + // also include references to that class. + N::getStaticBindingTargetFromRef(result) = this.getDeclaration().getNameNode() + } + + /** Gets the declaration of this type, if any. */ + ClassLikeDeclaration getDeclaration() { + result.getQualifiedName() = this and not exists(result.getExtensionTarget()) + } + + /** Gets the declaration of a proper subtype of this type (not the type itself). */ + ClassLikeDeclaration getASubtypeDeclaration() { + result.getABaseClass*().getABaseType().getType() = this.getAStaticRef() + } + + /** Gets a `Type` corresponding to instances of this type. */ + T::Type getAnInstanceType(boolean subtype) { + subtype = false and + ( + result.(Types::ModelType).getTypeSelector() = this + or + result.asClass() = this.getDeclaration() + ) + or + subtype = true and + result.asClass() = this.getASubtypeDeclaration() + } + + /** + * Gets a call to the constructor of this type. + * + * Used as a seed for type inference for when the constructor itself is not in the database. + */ + CallExpr getAConstructorCall() { constructorCallSelector(result, this, _) } +} + private predicate isAccessPath(string path) { legacySourceModel(_, _, _, _, _, _, path, _, _) // output or @@ -269,27 +394,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) ) } @@ -301,7 +456,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) ) } @@ -321,6 +476,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) + ) ) } @@ -340,6 +500,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/Type.qll b/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll index 0ffe9fccdd7a..70a91933f6d5 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll @@ -7,6 +7,7 @@ private import unified as Unified private import TypeMention private import TypeInference private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.mad.LegacyMaD as MaD /** * Holds if `a` is an associated type of `c`, in which case we model it @@ -70,6 +71,7 @@ newtype TType = associatedTypeParameter(c, a, inherited) } or TParentTypeParameterType(Member m) { isNestedType(m, _, _) } or + TModelType(MaD::TypeSelector selector) { not exists(selector.getDeclaration()) } or TUnknownType() or TUnknownTypeTypeParameter(int i) { i in [0 .. 20] } @@ -173,6 +175,26 @@ class AliasType extends TypeImpl, TAliasType { override Location getLocation() { result = alias.getLocation() } } +/** + * A type described by a type selector in a models-as-data row. + */ +class ModelType extends TypeImpl, TModelType { + private MaD::TypeSelector selector; + + ModelType() { this = TModelType(selector) } + + /** Gets the type selector describing this type. */ + MaD::TypeSelector getTypeSelector() { result = selector } + + override TypeParameter getPositionalTypeParameter(int i) { none() } + + override Identifier getNameNode() { none() } + + override string toString() { result = selector } + + override Location getLocation() { result instanceof EmptyLocation } +} + /** * A pseudo type that does not correspond to any concrete type in the source code. */ diff --git a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll index f7a773c1dc6d..8b5df77e75d9 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll @@ -334,14 +334,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() } @@ -807,6 +812,9 @@ module Input3 implements InputSig3 { or needsContextualTyping(n, path) and result instanceof UnknownType + or + path.isEmpty() and + n = result.(ModelType).getTypeSelector().getAConstructorCall() } predicate allowContextualInference(TypeMention tm, TypePath path) { @@ -886,6 +894,17 @@ predicate inferTypeUnique = M3::inferTypeUnique/2; predicate inferTypeCertain = M3::inferTypeCertain/2; +Type inferVariableType(LocalVariable variable) { + result = Input3::inferTypeFromVariable(variable, _) +} + +/** Gets the type of the given base of `access`. */ +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/lib/codeql/unified/internal/typeinference/TypeMention.qll b/unified/ql/lib/codeql/unified/internal/typeinference/TypeMention.qll index 5beb019ee80b..702a7351f276 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/TypeMention.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/TypeMention.qll @@ -280,6 +280,8 @@ class ExprTypeMention extends TypeMentionImpl, Expr { result = Plugin::getFunctionExprType(this) or result.(Plugin::TupleType).getArity() = this.(TupleExpr).getNumberOfElements() + or + result.(ModelType).getTypeSelector().getAStaticRef() = this } override Type getUnexpandedType() { diff --git a/unified/ql/src/queries/security/CWE-022/PathInjection.ql b/unified/ql/src/queries/security/CWE-022/PathInjection.ql index 20779510fc1b..54e661058287 100644 --- a/unified/ql/src/queries/security/CWE-022/PathInjection.ql +++ b/unified/ql/src/queries/security/CWE-022/PathInjection.ql @@ -39,7 +39,7 @@ predicate heuristicSink(DataFlow::Node node) { } module PathInjectionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { Models::isSource(node, _) } + predicate isSource(DataFlow::Node node) { Models::isSource(node, ["local", "remote"]) } predicate isSink(DataFlow::Node node) { Models::isSink(node, "path-injection") or diff --git a/unified/ql/src/queries/security/CWE-078/CommandInjection.ql b/unified/ql/src/queries/security/CWE-078/CommandInjection.ql index c1268d2bde21..5efb33eb0be7 100644 --- a/unified/ql/src/queries/security/CWE-078/CommandInjection.ql +++ b/unified/ql/src/queries/security/CWE-078/CommandInjection.ql @@ -18,12 +18,9 @@ import unified * A taint configuration for tainted data that reaches a command injection sink. */ module CommandInjectionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { Models::isSource(node, _) } + predicate isSource(DataFlow::Node node) { Models::isSource(node, ["local", "remote"]) } - 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..5962215f217a --- /dev/null +++ b/unified/ql/test/library-tests/mad-builtins/test.expected @@ -0,0 +1,8 @@ +isSink +| test.swift:14:5:14:52 | ... .tlsMinimumSupportedProtocolVersion | tls-protocol-version | +| test.swift:16:5:16:22 | ... .launchPath | command-injection | +| test.swift:17:5:17:21 | ... .arguments | command-injection | +| test.swift:18:5:18:23 | ... .arguments | command-injection | +| test.swift:21:5:21:26 | ... .arguments | command-injection | +isSource +| test.swift:12:9:12:22 | ... .text | local | 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..cd23768e7149 --- /dev/null +++ b/unified/ql/test/library-tests/mad-builtins/test.swift @@ -0,0 +1,30 @@ +// No stubs are declared in this file, so the types below must be resolved via builtins or other means. + +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 +} + +extension Process { + func testImplicitSelfSink(string: String) { + launchPath = string // $ MISSING: isSink=command-injection + arguments = [string] // $ MISSING: isSink=command-injection + arguments.append(string) // $ MISSING: isSink=command-injection + } +} diff --git a/unified/ql/test/library-tests/mad/test.expected b/unified/ql/test/library-tests/mad/test.expected index aaa07763e1e3..2698afe3c3f9 100644 --- a/unified/ql/test/library-tests/mad/test.expected +++ b/unified/ql/test/library-tests/mad/test.expected @@ -6,18 +6,32 @@ 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:137:5:139:5 | [self arg] ... .sinkArgs(...) | shorthand-sink | -| test.swift:138:9:138:14 | string | shorthand-sink | +| 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:111:5:111:20 | ... .encryptionKey | encryption-key | +| test.swift:114:5:114:20 | ... .encryptionKey | encryption-key | +| 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 | +| test.swift:217:5:219:5 | [self arg] ... .sinkArgs(...) | shorthand-sink | +| test.swift:218:9:218:14 | string | shorthand-sink | isSource | test.swift:4:5:4:27 | String(...) | remote | | test.swift:5:5:5:44 | String(...) | remote | @@ -27,5 +41,10 @@ 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:129:5:133:5 | [self param] func ShorthandTestSub.sourceParams | shorthand-source | -| test.swift:130:11:130:11 | x | shorthand-source | +| 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 | +| test.swift:209:5:213:5 | [self param] func ShorthandTestSub.sourceParams | shorthand-source | +| test.swift:210:11:210:11 | x | shorthand-source | diff --git a/unified/ql/test/library-tests/mad/test.swift b/unified/ql/test/library-tests/mad/test.swift index ff6c6975bfac..04a9c3229660 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 // $ isSink=encryption-key // nested types are not yet resolved by type inference + + var c2 = configuration + c2.encryptionKey = key // $ isSink=encryption-key // nested types are not yet resolved by type inference } func testQualifiedConstructors( @@ -119,6 +171,34 @@ 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 + } +} + class ShorthandTest { func sourceParams(_ x: String) {} diff --git a/unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected b/unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected index 74768a2a4957..e3dc141caff6 100644 --- a/unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected +++ b/unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected @@ -72,6 +72,8 @@ | testPathInjection.swift:445:33:445:44 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:445:33:445:44 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | | testPathInjection.swift:447:40:447:51 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:447:40:447:51 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | | testPathInjection.swift:456:15:456:26 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:456:15:456:26 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | +| testPathInjection.swift:469:5:469:18 | ... .fileURL | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:469:5:469:18 | ... .fileURL | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | +| testPathInjection.swift:471:5:471:23 | ... .seedFilePath | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:471:5:471:23 | ... .seedFilePath | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | | testPathInjection.swift:482:22:482:33 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:482:22:482:33 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | | testPathInjection.swift:484:24:484:30 | buffer2 | testPathInjection.swift:477:22:477:87 | Data(...) | testPathInjection.swift:484:24:484:30 | buffer2 | This path depends on a $@. | testPathInjection.swift:477:22:477:87 | Data(...) | user-provided value | | testPathInjection.swift:486:25:486:36 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:486:25:486:36 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value | @@ -173,6 +175,8 @@ edges | testPathInjection.swift:341:9:341:17 | remoteUrl | testPathInjection.swift:401:34:401:42 | remoteUrl | provenance | | | testPathInjection.swift:341:9:341:17 | remoteUrl | testPathInjection.swift:421:26:421:34 | remoteUrl | provenance | | | testPathInjection.swift:341:9:341:17 | remoteUrl | testPathInjection.swift:436:25:436:33 | remoteUrl | provenance | | +| testPathInjection.swift:341:9:341:17 | remoteUrl | testPathInjection.swift:469:22:469:30 | remoteUrl | provenance | | +| testPathInjection.swift:341:9:341:17 | remoteUrl | testPathInjection.swift:471:27:471:35 | remoteUrl | provenance | | | testPathInjection.swift:341:33:341:44 | remoteString | testPathInjection.swift:341:9:341:17 | remoteUrl | provenance | | | testPathInjection.swift:342:9:342:19 | remoteNsUrl | testPathInjection.swift:410:26:410:36 | remoteNsUrl | provenance | | | testPathInjection.swift:342:9:342:19 | remoteNsUrl | testPathInjection.swift:412:52:412:62 | remoteNsUrl | provenance | | @@ -181,6 +185,8 @@ edges | testPathInjection.swift:426:42:426:53 | remoteString | testPathInjection.swift:426:42:426:72 | TypeCastExpr | provenance | | | testPathInjection.swift:430:39:430:50 | remoteString | testPathInjection.swift:430:15:430:51 | FilePath(...) | provenance | | | testPathInjection.swift:433:39:433:50 | remoteString | testPathInjection.swift:433:15:433:51 | FilePath(...) | provenance | | +| testPathInjection.swift:469:22:469:30 | remoteUrl | testPathInjection.swift:469:5:469:18 | ... .fileURL | provenance | | +| testPathInjection.swift:471:27:471:35 | remoteUrl | testPathInjection.swift:471:5:471:23 | ... .seedFilePath | provenance | | | testPathInjection.swift:477:9:477:18 | remoteData | testPathInjection.swift:479:5:479:14 | remoteData | provenance | | | testPathInjection.swift:477:22:477:87 | Data(...) | testPathInjection.swift:477:9:477:18 | remoteData | provenance | | | testPathInjection.swift:479:5:479:14 | remoteData | testPathInjection.swift:479:30:479:36 | [post] buffer2 | provenance | | @@ -308,6 +314,10 @@ nodes | testPathInjection.swift:445:33:445:44 | remoteString | semmle.label | remoteString | | testPathInjection.swift:447:40:447:51 | remoteString | semmle.label | remoteString | | testPathInjection.swift:456:15:456:26 | remoteString | semmle.label | remoteString | +| testPathInjection.swift:469:5:469:18 | ... .fileURL | semmle.label | ... .fileURL | +| testPathInjection.swift:469:22:469:30 | remoteUrl | semmle.label | remoteUrl | +| testPathInjection.swift:471:5:471:23 | ... .seedFilePath | semmle.label | ... .seedFilePath | +| testPathInjection.swift:471:27:471:35 | remoteUrl | semmle.label | remoteUrl | | testPathInjection.swift:477:9:477:18 | remoteData | semmle.label | remoteData | | testPathInjection.swift:477:22:477:87 | Data(...) | semmle.label | Data(...) | | testPathInjection.swift:479:5:479:14 | remoteData | semmle.label | remoteData | diff --git a/unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift b/unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift index d9cb5ecaaa30..ffa8712e47cf 100644 --- a/unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift +++ b/unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift @@ -466,9 +466,9 @@ func test(buffer1: UnsafeMutablePointer, buffer2: UnsafeMutablePointer