Skip to content

Commit 9d71aee

Browse files
authored
Merge branch 'main' into bazookamusic/avro-updated
2 parents 0f0c28b + c3a0b65 commit 9d71aee

375 files changed

Lines changed: 15656 additions & 15714 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: deprecated
3+
---
4+
* Models-as-data flow summaries now use fully qualified field names (for example, `MyNamespace::MyStruct::myField`) instead of unqualified field names such as `myField`. We recommend updating existing flow summaries to use fully qualified field names. Unqualified field names are still supported, but that support will be removed in a future release.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: breaking
3+
---
4+
* Removed support for using variables as sources and sinks in models-as-data. Users of this feature should convert such sources and sinks to models defined using the QL language.

cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -931,31 +931,6 @@ private Element interpretElement0(
931931
signature = "" and
932932
elementSpec(namespace, type, subtypes, name, signature, _)
933933
)
934-
or
935-
// Member variables
936-
elementSpec(namespace, type, subtypes, name, signature, _) and
937-
signature = "" and
938-
exists(Class namedClass, Class classWithMember, MemberVariable member |
939-
member.getName() = name and
940-
member = classWithMember.getAMember() and
941-
namedClass.hasQualifiedName(namespace, type) and
942-
result = member
943-
|
944-
// field declared in the named type or a subtype of it (or an extension of any)
945-
subtypes = true and
946-
classWithMember = namedClass.getADerivedClass*()
947-
or
948-
// field declared directly in the named type (or an extension of it)
949-
subtypes = false and
950-
classWithMember = namedClass
951-
)
952-
or
953-
// Global or namespace variables
954-
elementSpec(namespace, type, subtypes, name, signature, _) and
955-
signature = "" and
956-
type = "" and
957-
subtypes = false and
958-
result = any(GlobalOrNamespaceVariable v | v.hasQualifiedName(namespace, name))
959934
}
960935

961936
cached

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ private import cpp as Cpp
66
private import codeql.dataflow.internal.FlowSummaryImpl
77
private import codeql.dataflow.internal.AccessPathSyntax as AccessPath
88
private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
9+
private import semmle.code.cpp.ir.dataflow.internal.DataFlowNodes
910
private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
1011
private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplSpecific as DataFlowImplSpecific
1112
private import semmle.code.cpp.dataflow.ExternalFlow
@@ -20,8 +21,22 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
2021

2122
class SinkBase = Void;
2223

24+
class FlowSummaryCallBase = CallInstruction;
25+
2326
predicate callableFromSource(SummarizedCallableBase c) { exists(c.getBlock()) }
2427

28+
FlowSummaryCallBase getASourceCall(SummarizedCallableBase sc) {
29+
result.getStaticCallTarget() = sc
30+
}
31+
32+
DataFlowCallable getSummarizedCallableAsDataFlowCallable(SummarizedCallableBase c) {
33+
result.asSummarizedCallable() = c
34+
}
35+
36+
DataFlowCallable getSourceCallEnclosingCallable(FlowSummaryCallBase call) {
37+
result.asSourceCallable() = call.getEnclosingFunction()
38+
}
39+
2540
ArgumentPosition callbackSelfParameterPosition() { result = TDirectPosition(-1) }
2641

2742
ReturnKind getStandardReturnValueKind() { result = getReturnValueKind("") }
@@ -30,6 +45,10 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
3045
arg = repeatStars(result.(NormalReturnKind).getIndirectionIndex())
3146
}
3247

48+
ParameterPosition getFlowSummaryParameterPosition(ReturnKind rk) {
49+
result = TFlowSummaryPosition(rk)
50+
}
51+
3352
string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
3453

3554
string encodeArgumentPosition(ArgumentPosition pos) { result = pos.toString() }
@@ -40,12 +59,24 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
4059
arg = repeatStars(rk.(NormalReturnKind).getIndirectionIndex())
4160
}
4261

62+
bindingset[namespace, type, base]
63+
private string formatQualifiedName(string namespace, string type, string base) {
64+
if namespace = ""
65+
then result = type + "::" + base
66+
else result = namespace + "::" + type + "::" + base
67+
}
68+
4369
string encodeContent(ContentSet cs, string arg) {
44-
exists(FieldContent c |
70+
exists(FieldContent c, string namespace, string type, string base |
4571
cs.isSingleton(c) and
4672
// FieldContent indices have 0 for the address, 1 for content, so we need to subtract one.
4773
result = "Field" and
48-
arg = repeatStars(c.getIndirectionIndex() - 1) + c.getField().getName()
74+
c.getField().hasQualifiedName(namespace, type, base)
75+
|
76+
arg = repeatStars(c.getIndirectionIndex() - 1) + formatQualifiedName(namespace, type, base)
77+
or
78+
// TODO: This disjunct can be removed once we stop supporting unqualified field names.
79+
arg = repeatStars(c.getIndirectionIndex() - 1) + base
4980
)
5081
or
5182
exists(ElementContent ec |
@@ -102,10 +133,22 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
102133
private import Make<Location, DataFlowImplSpecific::CppDataFlow, Input> as Impl
103134

104135
private module StepsInput implements Impl::Private::StepsInputSig {
136+
Impl::Private::SummaryNode getSummaryNode(Node n) {
137+
result = n.(FlowSummaryNode).getSummaryNode()
138+
}
139+
105140
DataFlowCall getACall(Public::SummarizedCallable sc) {
106141
result.getStaticCallTarget().getUnderlyingCallable() = sc
107142
}
108143

144+
Node getSourceOutNode(Input::FlowSummaryCallBase call, ReturnKind rk) {
145+
exists(IndirectReturnOutNode out | result = out |
146+
out.getCallInstruction() = call and
147+
pragma[only_bind_out](rk.(NormalReturnKind).getIndirectionIndex()) =
148+
pragma[only_bind_out](out.getIndirectionIndex())
149+
)
150+
}
151+
109152
DataFlowCallable getSourceNodeEnclosingCallable(Input::SourceBase source) { none() }
110153

111154
Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponentStack s) { none() }
@@ -218,40 +261,11 @@ module SourceSinkInterpretationInput implements
218261

219262
/** Provides additional sink specification logic. */
220263
bindingset[c]
221-
predicate interpretOutput(string c, InterpretNode mid, InterpretNode node) {
222-
// Allow variables to be picked as output nodes.
223-
exists(Node n, Element ast |
224-
n = node.asNode() and
225-
ast = mid.asElement()
226-
|
227-
c = "" and
228-
n.asExpr().(VariableAccess).getTarget() = ast
229-
)
230-
}
264+
predicate interpretOutput(string c, InterpretNode mid, InterpretNode node) { none() }
231265

232266
/** Provides additional source specification logic. */
233267
bindingset[c]
234-
predicate interpretInput(string c, InterpretNode mid, InterpretNode node) {
235-
exists(Node n, Element ast, VariableAccess e |
236-
n = node.asNode() and
237-
ast = mid.asElement() and
238-
e.getTarget() = ast
239-
|
240-
// Allow variables to be picked as input nodes.
241-
// We could simply do this as `e = n.asExpr()`, but that would not allow
242-
// us to pick `x` as a sink in an example such as `x = source()` (but
243-
// only subsequent uses of `x`) since the variable access on `x` doesn't
244-
// actually load the value of `x`. So instead, we pick the instruction
245-
// node corresponding to the generated `StoreInstruction` and use the
246-
// expression associated with the destination instruction. This means
247-
// that the `x` in `x = source()` can be marked as an input.
248-
c = "" and
249-
exists(StoreInstruction store |
250-
store.getDestinationAddress().getUnconvertedResultExpression() = e and
251-
n.asInstruction() = store
252-
)
253-
)
254-
}
268+
predicate interpretInput(string c, InterpretNode mid, InterpretNode node) { none() }
255269
}
256270

257271
module Private {

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,12 +1534,8 @@ class FlowSummaryNode extends Node, TFlowSummaryNode {
15341534
result = this.getSummaryNode().getSummarizedCallable()
15351535
}
15361536

1537-
/**
1538-
* Gets the enclosing callable. For a `FlowSummaryNode` this is always the
1539-
* summarized function this node is part of.
1540-
*/
15411537
override DataFlowCallable getEnclosingCallable() {
1542-
result.asSummarizedCallable() = this.getSummarizedCallable()
1538+
result = FlowSummaryImpl::Private::getEnclosingCallable(this.getSummaryNode())
15431539
}
15441540

15451541
override Location getLocationImpl() { result = this.getSummarizedCallable().getLocation() }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,21 @@ class SummaryArgumentNode extends ArgumentNode, FlowSummaryNode {
561561
}
562562
}
563563

564+
/** An argument node that re-enters return output as input to a flow summary. */
565+
private class FlowSummaryArgumentNode extends ArgumentNode, FlowSummaryNode {
566+
private CallInstruction callInstruction;
567+
private ReturnKind rk;
568+
569+
FlowSummaryArgumentNode() {
570+
this.getSummaryNode() = FlowSummaryImpl::Private::summaryArgumentNode(callInstruction, rk)
571+
}
572+
573+
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
574+
call.asCallInstruction() = callInstruction and
575+
pos = TFlowSummaryPosition(rk)
576+
}
577+
}
578+
564579
/** A parameter position represented by an integer. */
565580
class ParameterPosition = Position;
566581

@@ -616,6 +631,18 @@ class IndirectionPosition extends Position, TIndirectionPosition {
616631
final override int getIndirectionIndex() { result = indirectionIndex }
617632
}
618633

634+
class FlowSummaryPosition extends Position, TFlowSummaryPosition {
635+
ReturnKind rk;
636+
637+
FlowSummaryPosition() { this = TFlowSummaryPosition(rk) }
638+
639+
override string toString() { result = "write to: " + rk.toString() }
640+
641+
override int getArgumentIndex() { none() }
642+
643+
final override int getIndirectionIndex() { result = rk.getIndirectionIndex() }
644+
}
645+
619646
newtype TPosition =
620647
TDirectPosition(int argumentIndex) {
621648
exists(any(CallInstruction c).getArgument(argumentIndex))
@@ -634,7 +661,8 @@ newtype TPosition =
634661
p = f.getParameter(argumentIndex) and
635662
indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1]
636663
)
637-
}
664+
} or
665+
TFlowSummaryPosition(ReturnKind rk) { FlowSummaryImpl::Private::relevantFlowSummaryPosition(rk) }
638666

639667
private newtype TReturnKind =
640668
TNormalReturnKind(int indirectionIndex) {
@@ -1378,6 +1406,8 @@ predicate nodeIsHidden(Node n) {
13781406
n instanceof InitialGlobalValue
13791407
or
13801408
n instanceof SsaSynthNode
1409+
or
1410+
n.(FlowSummaryNode).getSummaryNode().isHidden()
13811411
}
13821412

13831413
predicate neverSkipInPathGraph(Node n) {

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private module Cached {
158158
model = ""
159159
or
160160
// models-as-data summarized flow
161-
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom.(FlowSummaryNode).getSummaryNode(),
161+
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom,
162162
nodeTo.(FlowSummaryNode).getSummaryNode(), true, model)
163163
}
164164

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private module Cached {
6767
model = ""
6868
or
6969
// models-as-data summarized flow
70-
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom.(FlowSummaryNode).getSummaryNode(),
70+
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom,
7171
nodeTo.(FlowSummaryNode).getSummaryNode(), false, model)
7272
or
7373
// object->field conflation for content that is a `TaintInheritingContent`.

0 commit comments

Comments
 (0)