diff --git a/go/ql/lib/semmle/go/Types.qll b/go/ql/lib/semmle/go/Types.qll index 1b09ea466cc4..8abd94776cc3 100644 --- a/go/ql/lib/semmle/go/Types.qll +++ b/go/ql/lib/semmle/go/Types.qll @@ -483,7 +483,7 @@ class StructType extends @structtype, CompositeType { /** * Holds if there is an embedded field at `depth`, with either type `tp` or a pointer to `tp`. */ - private predicate hasEmbeddedField(Type tp, int depth) { + predicate hasEmbeddedField(Type tp, int depth) { exists(Field f | this.hasFieldCand(_, f, depth, true) | tp = f.getType() or tp = f.getType().(PointerType).getBaseType() diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll index 68a876deaea6..c557b5af7de3 100644 --- a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -475,25 +475,21 @@ SourceSinkInterpretationInput::SourceOrSinkElement interpretElement( // Go does not need to distinguish functions with signature signature = "" and exists(string p | p = interpretPackage(pkg) | - exists(Field f | f.hasQualifiedName(p, type, name) | - result.asEntity() = f and - result.hasTypeInfo(p, type, subtypes) + result.hasTypeInfo(p, type, subtypes) and + ( + result.asEntity().(Field).hasQualifiedName(p, type, name) or + result.asEntity().(Method).hasQualifiedName(p, type, name) ) or - exists(Method m | m.hasQualifiedName(p, type, name) | - result.asEntity() = m and - result.hasTypeInfo(p, type, subtypes) - or - subtypes = true and - // p.type is an interface and we include types which implement it - exists(Method m2, string pkg2, string type2 | - m2.getReceiverType().implements(p, type) and - m2.getName() = name and - m2.getReceiverBaseType().hasQualifiedName(pkg2, type2) - | - result.asEntity() = m2 and - result.hasTypeInfo(pkg2, type2, subtypes) - ) + subtypes = true and + // p.type is an interface and we include types which implement it + exists(Method m2, string pkg2, string type2 | + m2.getReceiverType().implements(p, type) and + m2.getName() = name and + m2.getReceiverBaseType().hasQualifiedName(pkg2, type2) + | + result.asEntity() = m2 and + result.hasTypeInfo(pkg2, type2, subtypes) ) or type = "" and diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 14a92748d526..e54fb5665042 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -149,6 +149,9 @@ module SourceSinkInterpretationInput implements ) } + // Note that due to embedding, which is currently implemented via some Methods + // or Fields having multiple qualified names, a given Method or Field is liable + // to have more than one SourceOrSinkElement, one for each of the names it claims. private newtype TSourceOrSinkElement = TMethodEntityElement(Method m, string pkg, string type, boolean subtypes) { m.hasQualifiedName(pkg, type, _) and subtypes = [true, false] @@ -175,7 +178,7 @@ module SourceSinkInterpretationInput implements AstNode asAstNode() { this = TAstElement(result) } /** - * Holds if this source or sink element is a method that was specified + * Holds if this source or sink element is a method or field that was specified * with the given values for `pkg`, `type` and `subtypes`. */ predicate hasTypeInfo(string pkg, string type, boolean subtypes) { @@ -241,7 +244,7 @@ module SourceSinkInterpretationInput implements ( not callTarget instanceof Method or - ensureCorrectTypeInfo(result, cn.getReceiver()) + elementAppliesToQualifier(result, cn.getReceiver()) ) ) } @@ -268,55 +271,54 @@ module SourceSinkInterpretationInput implements } } - private predicate ensureCorrectTypeInfo(SourceOrSinkElement sse, DataFlow::Node recv) { + /** + * Holds if method or field spec `sse` applies in the context of qualifier `qual`. + * + * Note that naively checking `sse.asEntity()`'s qualified name is not correct, because + * `Method`s and `Field`s may have multiple qualified names due to embedding. We must instead + * check that the specific name given be `sse.hasTypeInfo` refers to either `qual`'s type + * or to a type it embeds. + */ + private predicate elementAppliesToQualifier(SourceOrSinkElement sse, DataFlow::Node qual) { ( - exists(DataFlow::CallNode cn | cn.getReceiver() = recv and cn.getTarget() = sse.asEntity()) + exists(DataFlow::CallNode cn | cn.getReceiver() = qual and cn.getTarget() = sse.asEntity()) or - exists(DataFlow::FieldReadNode frn | frn.getBase() = recv and frn.getField() = sse.asEntity()) + exists(DataFlow::FieldReadNode frn | frn.getBase() = qual and frn.getField() = sse.asEntity()) or - exists(DataFlow::Write fw | fw.writesField(recv, sse.asEntity(), _)) + exists(DataFlow::Write fw | fw.writesField(qual, sse.asEntity(), _)) ) and - exists(string pkg, string typename, boolean subtypes, Type syntacticRecvBaseType | + exists( + string pkg, string typename, boolean subtypes, Type syntacticQualBaseType, Type targetType + | sse.hasTypeInfo(pkg, typename, subtypes) and - syntacticRecvBaseType = lookThroughPointerType(getSyntacticRecv(recv).getType()) + targetType.hasQualifiedName(pkg, typename) and + syntacticQualBaseType = lookThroughPointerType(getSyntacticQualifier(qual).getType()) | subtypes = [true, false] and - syntacticRecvBaseType.hasQualifiedName(pkg, typename) + syntacticQualBaseType = targetType or subtypes = true and ( - // `syntacticRecvBaseType`'s underlying type might be an interface type and `sse` - // might be a method defined on an interface which is a subtype of it. - exists(Type t | - t = syntacticRecvBaseType.getUnderlyingType().(InterfaceType).getAnEmbeddedInterface() and - t.hasQualifiedName(pkg, typename) and - sse.asEntity().(Method).hasQualifiedName(pkg, typename, _) - ) + // `syntacticQualBaseType`'s underlying type might be an interface type and `sse` + // might refer to a method defined on an interface embedded within it. + targetType = + syntacticQualBaseType.getUnderlyingType().(InterfaceType).getAnEmbeddedInterface() or - // `syntacticRecvBaseType`'s underlying type might be a struct type and `sse` - // might be a promoted method or field. - exists(StructType st, Field field1, Field field2, int depth1, int depth2, Type t1, Type t2 | - st = syntacticRecvBaseType.getUnderlyingType() and - field1 = st.getFieldAtDepth(_, depth1) and - field2 = st.getFieldAtDepth(_, depth2) and - t1 = lookThroughPointerType(field1.getType()) and - t2 = lookThroughPointerType(field2.getType()) and - ( - field1 = field2 - or - field2 = t1.getUnderlyingType().(StructType).getFieldAtDepth(_, depth2 - depth1 - 1) - ) and - matchTypeInfo(sse, t1) - | - sse.asEntity().(Method).getReceiverBaseType() = t2 - or - sse.asEntity().(Field).getDeclaringType() = t2.getUnderlyingType() - ) + // `syntacticQualBaseType`'s underlying type might be a struct type and `sse` + // might refer to an embedded method or field. + syntacticQualBaseType.getUnderlyingType().(StructType).hasEmbeddedField(targetType, _) ) ) } - private DataFlow::Node getSyntacticRecv(DataFlow::Node n) { + /** + * Gets `underlying`, where `n` if of the form `implicitDeref?(underlying.implicitFieldRead1.implicitFieldRead2...)` + * + * For Go syntax like `qualifier.method()` or `qualifier.field`, this is the type of `qualifier`, before any + * implicit dereference is interposed because `qualifier` is of pointer type, or implicit field accesses + * navigate to any embedded struct types that truly host `field`. + */ + private DataFlow::Node getSyntacticQualifier(DataFlow::Node n) { exists(DataFlow::Node n2 | // look through implicit dereference, if there is one not exists(n.asInstruction().(IR::EvalImplicitDerefInstruction).getOperand()) and @@ -329,13 +331,12 @@ module SourceSinkInterpretationInput implements } private DataFlow::Node skipImplicitFieldReads(DataFlow::Node n) { - not exists(getImplicitFieldReadInstruction(n)) and result = n + not exists(lookThroughImplicitFieldRead(n)) and result = n or - result = skipImplicitFieldReads(getImplicitFieldReadInstruction(n)) + result = skipImplicitFieldReads(lookThroughImplicitFieldRead(n)) } - pragma[inline] - private DataFlow::Node getImplicitFieldReadInstruction(DataFlow::Node n) { + private DataFlow::Node lookThroughImplicitFieldRead(DataFlow::Node n) { result.asInstruction() = n.(DataFlow::InstructionNode) .asInstruction() @@ -343,15 +344,6 @@ module SourceSinkInterpretationInput implements .getBaseInstruction() } - bindingset[sse, t] - pragma[inline_late] - private predicate matchTypeInfo(SourceOrSinkElement sse, Type t) { - exists(string pkg, string typename | - sse.hasTypeInfo(pkg, typename, true) and - t.hasQualifiedName(pkg, typename) - ) - } - /** Provides additional sink specification logic. */ bindingset[c] predicate interpretOutput(string c, InterpretNode mid, InterpretNode node) { @@ -371,7 +363,7 @@ module SourceSinkInterpretationInput implements exists(DataFlow::FieldReadNode frn | frn = n | c = "" and frn.getField() = e.asEntity() and - ensureCorrectTypeInfo(e, frn.getBase()) + elementAppliesToQualifier(e, frn.getBase()) ) ) } @@ -392,7 +384,7 @@ module SourceSinkInterpretationInput implements | c = "" and fw.writesField(base, f, node.asNode()) and - ensureCorrectTypeInfo(e, base) + elementAppliesToQualifier(e, base) ) } }