From d0673a1c1861691d37daf6ffc17c07d69ba06e12 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 12 Dec 2024 19:15:19 +0100 Subject: [PATCH] wip --- .../frameworks/GoMicro/LogInjection.expected | 46 ++++++ .../dataflow/internal/DataFlowImplCommon.qll | 133 ++++++++++++------ 2 files changed, 134 insertions(+), 45 deletions(-) diff --git a/go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected b/go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected index b5614b13a45c6..06c99ce736bc9 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected +++ b/go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected @@ -1,26 +1,72 @@ edges | main.go:18:46:18:48 | definition of req | main.go:18:46:18:48 | definition of req [Reverse] | provenance | | +| main.go:18:46:18:48 | definition of req | main.go:20:10:20:12 | implicit dereference | provenance | | +| main.go:18:46:18:48 | definition of req | main.go:20:10:20:12 | implicit dereference | provenance | | +| main.go:18:46:18:48 | definition of req | main.go:21:28:21:31 | name | provenance | | | main.go:18:46:18:48 | definition of req | main.go:21:28:21:31 | name | provenance | | | main.go:18:46:18:48 | definition of req | main.go:21:28:21:31 | name | provenance | | | main.go:18:46:18:48 | definition of req [Reverse] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | provenance | | +| main.go:18:46:18:48 | definition of req [Reverse] [pointer, Name] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | provenance | | +| main.go:18:46:18:48 | definition of req [pointer, Name] | main.go:20:10:20:12 | req [pointer, Name] | provenance | | +| main.go:20:10:20:12 | implicit dereference | main.go:18:46:18:48 | definition of req [Reverse] | provenance | | +| main.go:20:10:20:12 | implicit dereference | main.go:21:28:21:31 | name | provenance | | +| main.go:20:10:20:12 | implicit dereference | main.go:21:28:21:31 | name | provenance | | +| main.go:20:10:20:12 | implicit dereference | main.go:21:28:21:31 | name | provenance | | +| main.go:20:10:20:12 | implicit dereference [Name] | main.go:20:10:20:17 | selection of Name | provenance | | +| main.go:20:10:20:12 | implicit dereference [Reverse] [Name] | main.go:20:10:20:12 | req [Reverse] [pointer, Name] | provenance | | +| main.go:20:10:20:12 | req [Reverse] [pointer, Name] | main.go:18:46:18:48 | definition of req [Reverse] [pointer, Name] | provenance | | +| main.go:20:10:20:12 | req [pointer, Name] | main.go:20:10:20:12 | implicit dereference [Name] | provenance | | +| main.go:20:10:20:17 | selection of Name | main.go:21:28:21:31 | name | provenance | | +| main.go:20:10:20:17 | selection of Name [Reverse] | main.go:20:10:20:12 | implicit dereference [Reverse] [Name] | provenance | | +| main.go:21:2:21:32 | []type{args} [Reverse] [array] | main.go:21:28:21:31 | name [Reverse] | provenance | | +| main.go:21:2:21:32 | []type{args} [array] | main.go:21:2:21:32 | []type{args} [Reverse] [array] | provenance | | +| main.go:21:28:21:31 | name | main.go:21:2:21:32 | []type{args} [array] | provenance | | +| main.go:21:28:21:31 | name [Reverse] | main.go:20:10:20:17 | selection of Name [Reverse] | provenance | | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] | provenance | | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | proto/Hello.pb.micro.go:86:37:86:38 | in | provenance | | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | proto/Hello.pb.micro.go:86:37:86:38 | in | provenance | | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | provenance | | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] [pointer, Name] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | provenance | | | proto/Hello.pb.micro.go:86:37:86:38 | in | main.go:18:46:18:48 | definition of req | provenance | | | proto/Hello.pb.micro.go:86:37:86:38 | in | main.go:18:46:18:48 | definition of req | provenance | | | proto/Hello.pb.micro.go:86:37:86:38 | in | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | provenance | | | proto/Hello.pb.micro.go:86:37:86:38 | in | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | provenance | | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | main.go:18:46:18:48 | definition of req [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | main.go:18:46:18:48 | definition of req [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | provenance | | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | provenance | | nodes | main.go:18:46:18:48 | definition of req | semmle.label | definition of req | | main.go:18:46:18:48 | definition of req | semmle.label | definition of req | | main.go:18:46:18:48 | definition of req [Reverse] | semmle.label | definition of req [Reverse] | +| main.go:18:46:18:48 | definition of req [Reverse] [pointer, Name] | semmle.label | definition of req [Reverse] [pointer, Name] | +| main.go:18:46:18:48 | definition of req [pointer, Name] | semmle.label | definition of req [pointer, Name] | +| main.go:20:10:20:12 | implicit dereference | semmle.label | implicit dereference | +| main.go:20:10:20:12 | implicit dereference | semmle.label | implicit dereference | +| main.go:20:10:20:12 | implicit dereference [Name] | semmle.label | implicit dereference [Name] | +| main.go:20:10:20:12 | implicit dereference [Reverse] [Name] | semmle.label | implicit dereference [Reverse] [Name] | +| main.go:20:10:20:12 | req [Reverse] [pointer, Name] | semmle.label | req [Reverse] [pointer, Name] | +| main.go:20:10:20:12 | req [pointer, Name] | semmle.label | req [pointer, Name] | +| main.go:20:10:20:17 | selection of Name | semmle.label | selection of Name | +| main.go:20:10:20:17 | selection of Name [Reverse] | semmle.label | selection of Name [Reverse] | +| main.go:21:2:21:32 | []type{args} [Reverse] [array] | semmle.label | []type{args} [Reverse] [array] | +| main.go:21:2:21:32 | []type{args} [array] | semmle.label | []type{args} [array] | +| main.go:21:28:21:31 | name | semmle.label | name | | main.go:21:28:21:31 | name | semmle.label | name | +| main.go:21:28:21:31 | name [Reverse] | semmle.label | name [Reverse] | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | semmle.label | definition of in | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in | semmle.label | definition of in | | proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] | semmle.label | definition of in [Reverse] | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [Reverse] [pointer, Name] | semmle.label | definition of in [Reverse] [pointer, Name] | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | semmle.label | definition of in [pointer, Name] | +| proto/Hello.pb.micro.go:85:53:85:54 | definition of in [pointer, Name] | semmle.label | definition of in [pointer, Name] | | proto/Hello.pb.micro.go:86:37:86:38 | in | semmle.label | in | | proto/Hello.pb.micro.go:86:37:86:38 | in | semmle.label | in | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | semmle.label | in [pointer, Name] | +| proto/Hello.pb.micro.go:86:37:86:38 | in [pointer, Name] | semmle.label | in [pointer, Name] | subpaths #select | main.go:21:28:21:31 | name | main.go:18:46:18:48 | definition of req | main.go:21:28:21:31 | name | This log entry depends on a $@. | main.go:18:46:18:48 | definition of req | user-provided value | diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index f3b3a07b44a6b..a169df2249cf8 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -878,15 +878,19 @@ module MakeImplCommon Lang> { string toString() { result = this.asNode().toString() or - exists(Node n | this.isImplicitReadNode(n) | result = n.toString() + " [Ext]") + exists(Node n | this.isImplicitReadNode(n) | result = n + " [Ext]") or - result = this.asNodeReverse(_).toString() + " [Reverse]" + result = this.asNodeReverse(_) + " [Reverse]" + or + result = this.asNodeReverseReturnPosition() + " [ReverseReturn]" } Node asNode() { this = TNodeNormal(result) } Node asNodeReverse(boolean allowFwdFlowOut) { this = TNodeReverse(result, allowFwdFlowOut) } + ReturnPosition asNodeReverseReturnPosition() { this = TNodeReverseReturn(result) } + /** Gets the corresponding Node if this is a normal node or its post-implicit read node. */ Node asNodeOrImplicitRead() { this = TNodeNormal(result) or this = TNodeImplicitRead(result) } @@ -901,6 +905,8 @@ module MakeImplCommon Lang> { pragma[nomagic] private DataFlowCallable getEnclosingCallable0() { nodeEnclosingCallable(this.projectToNode(), result) + or + result = this.asNodeReverseReturnPosition().getCallable() } pragma[inline] @@ -915,6 +921,8 @@ module MakeImplCommon Lang> { nodeDataFlowType(this.asNodeReverse(_), result) or isTopType(result) and this.isImplicitReadNode(_) + or + result = this.(ReverseFlow::ReverseParamNodeEx).getAReturnNode().getDataFlowType0() } pragma[inline] @@ -1047,6 +1055,7 @@ module MakeImplCommon Lang> { private module ReverseFlow { module Cand { + // todo: check types /** * Holds if `p` can flow to `node` in the same callable using only * value-preserving steps. @@ -1054,45 +1063,63 @@ module MakeImplCommon Lang> { * `read` indicates whether it is contents of `p` that can flow to `node`. */ pragma[nomagic] - private predicate parameterValueFlowCand(ParamNode p, Node node) { - ( - p = node - or - // local flow - exists(Node mid | - parameterValueFlowCand(p, mid) and - simpleLocalFlowStep(mid, node, _) and - validParameterAliasStep(mid, node) - ) - or - // store - exists(Node mid | - parameterValueFlowCand(p, mid) and - Lang::storeStep(mid, _, node) - ) - or - // read - exists(Node mid | - parameterValueFlowCand(p, mid) and - Lang::readStep(mid, _, node) - ) - or - // flow through - exists(ArgNode arg | - parameterValueFlowArgCand(p, arg) and - argumentValueFlowsThroughCand(arg, node) - ) + private predicate parameterValueFlowCand(ParamNode p, DataFlowType t, Node node) { + parameterValueFlowCand0(p, t, node) and + if node instanceof CastingNode + then compatibleTypesFilter(t, getNodeDataFlowType(node)) + else any() + } + + /** + * Holds if `p` can flow to `node` in the same callable using only + * value-preserving steps. + * + * `read` indicates whether it is contents of `p` that can flow to `node`. + */ + pragma[nomagic] + private predicate parameterValueFlowCand0(ParamNode p, DataFlowType t, Node node) { + p = node and + t = getNodeDataFlowType(node) + or + // local flow + exists(Node mid | + parameterValueFlowCand(p, t, mid) and + simpleLocalFlowStep(mid, node, _) and + validParameterAliasStep(mid, node) + ) + or + // store + exists(Node mid, DataFlowType t0 | + parameterValueFlowCand(p, t0, mid) and + Lang::storeStep(mid, _, node) and + t = getNodeDataFlowType(node) and + compatibleTypes(t0, pragma[only_bind_out](getNodeDataFlowType(mid))) + ) + or + // read + exists(Node mid, DataFlowType t0 | + parameterValueFlowCand(p, t0, mid) and + Lang::readStep(mid, _, node) and + t = getNodeDataFlowType(node) and + compatibleTypes(t0, pragma[only_bind_out](getNodeDataFlowType(mid))) + ) + or + // flow through + exists(ParamNode p0, DataFlowType t0, ArgNode arg | + parameterValueFlowArgCand(p, t0, arg) and + argumentValueFlowsThroughCand(arg, p0, t, node) and + compatibleTypes(t0, getNodeDataFlowType(p0)) ) } pragma[nomagic] - private predicate parameterValueFlowArgCand(ParamNode p, ArgNode arg) { - parameterValueFlowCand(p, arg) + private predicate parameterValueFlowArgCand(ParamNode p, DataFlowType t, ArgNode arg) { + parameterValueFlowCand(p, t, arg) } pragma[nomagic] predicate parameterValueFlowsToPreUpdateCand(ParamNode p, PostUpdateNode n) { - parameterValueFlowCand(p, n.getPreUpdateNode()) + parameterValueFlowCand(p, _, n.getPreUpdateNode()) } /** @@ -1103,20 +1130,19 @@ module MakeImplCommon Lang> { * `read` indicates whether it is contents of `p` that can flow to the return * node. */ - predicate parameterValueFlowReturnCand(ParamNode p, ReturnKind kind) { + predicate parameterValueFlowReturnCand(ParamNode p, DataFlowType t, ReturnKind kind) { exists(ReturnNode ret | - parameterValueFlowCand(p, ret) and + parameterValueFlowCand(p, t, ret) and kind = ret.getKind() ) } pragma[nomagic] private predicate argumentValueFlowsThroughCand0( - DataFlowCall call, ArgNode arg, ReturnKind kind + DataFlowCall call, ParamNode param, ArgNode arg, DataFlowType t, ReturnKind kind ) { - exists(ParamNode param | viableParamArg(call, param, arg) | - parameterValueFlowReturnCand(param, kind) - ) + viableParamArg(call, param, arg) and + parameterValueFlowReturnCand(param, t, kind) } /** @@ -1125,17 +1151,17 @@ module MakeImplCommon Lang> { * * `read` indicates whether it is contents of `arg` that can flow to `out`. */ - predicate argumentValueFlowsThroughCand(ArgNode arg, Node out) { + predicate argumentValueFlowsThroughCand(ArgNode arg, ParamNode p, DataFlowType t, Node out) { exists(DataFlowCall call, ReturnKind kind | - argumentValueFlowsThroughCand0(call, arg, kind) and + argumentValueFlowsThroughCand0(call, arg, p, t, kind) and out = getAnOutNode(call, kind) ) } predicate cand(ParamNode p, Node n) { - parameterValueFlowCand(p, n) and + parameterValueFlowCand(p, _, n) and ( - parameterValueFlowReturnCand(p, _) + parameterValueFlowReturnCand(p, _, _) or parameterValueFlowsToPreUpdateCand(p, _) ) @@ -1178,6 +1204,9 @@ module MakeImplCommon Lang> { node2.asNode().(PostUpdateNode).getPreUpdateNode() = n2 ) or + node2 = node1.(ReverseParamNodeEx).getAReturnNode() and + model = "" + or node1.asNode().(PostUpdateNode).getPreUpdateNode() = node2.asNodeReverse(true) and model = "" } @@ -1240,18 +1269,23 @@ module MakeImplCommon Lang> { } } - final class ReverseParamNodeEx extends ParamNodeEx { + final class ReverseParamNodeEx extends ParamNodeEx, TNodeReverseReturn { private DataFlowCallable c_; private ParameterPositionEx pos_; ReverseParamNodeEx() { exists(ReturnPosition pos | - pos = getValueReturnPosition(this.asNodeReverse(false)) and + this = TNodeReverseReturn(pos) and + // pos = getValueReturnPosition(this.asNodeReverse(false)) and c_ = pos.getCallable() and pos_.asReturnKind() = pos.getKind() ) } + NodeEx getAReturnNode() { + this = TNodeReverseReturn(getValueReturnPosition(result.asNodeReverse(false))) + } + override predicate isParameterOf(DataFlowCallable c, ParameterPositionEx pos) { c = c_ and pos = pos_ } @@ -1349,6 +1383,8 @@ module MakeImplCommon Lang> { nodeIsHidden([n.asNode(), n.asNodeReverse(_)]) or n instanceof TNodeImplicitRead + or + n instanceof ReverseFlow::ReverseParamNodeEx } cached @@ -2191,6 +2227,13 @@ module MakeImplCommon Lang> { TNodeReverse(Node n, Boolean allowFwdFlowOut) { (ReverseFlow::Cand::cand(_, n) or n = any(PostUpdateNode p).getPreUpdateNode()) and if allowFwdFlowOut = false then ReverseFlow::Cand::cand(_, n) else any() + } or + TNodeReverseReturn(ReturnPosition pos) { + exists(ParamNode p, ReturnKind kind | + pos.getKind().(ValueReturnKind).getKind() = kind and + ReverseFlow::Cand::parameterValueFlowReturnCand(p, _, kind) and + pos.getCallable() = getNodeEnclosingCallable(p) + ) } /**