Skip to content

Commit 21ff705

Browse files
owen-mcowen-mc
committed
Fix bug with read/store steps and named types
1 parent 1af3374 commit 21ff705

File tree

3 files changed

+68
-64
lines changed

3 files changed

+68
-64
lines changed

‎go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll

Lines changed: 61 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -14,38 +14,40 @@ private import semmle.go.dataflow.ExternalFlow
1414
* varargs.
1515
*/
1616
predicate containerStoreStep(Node node1, Node node2, Content c) {
17-
c instanceof ArrayContent and
18-
(
17+
exists(Type t | t = node2.getType().getUnderlyingType() |
18+
c instanceof ArrayContent and
1919
(
20-
node2.getType() instanceof ArrayType or
21-
node2.getType() instanceof SliceType
22-
) and
23-
(
24-
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
25-
or
26-
node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
27-
or
28-
// To model data flow from array elements of the base of a `SliceNode` to
29-
// the `SliceNode` itself, we consider there to be a read step with array
30-
// content from the base to the corresponding `SliceElementNode` and then
31-
// a store step with array content from the `SliceelementNode` to the
32-
// `SliceNode` itself.
33-
node2 = node1.(SliceElementNode).getSliceNode()
20+
(
21+
t instanceof ArrayType or
22+
t instanceof SliceType
23+
) and
24+
(
25+
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
26+
or
27+
node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
28+
or
29+
// To model data flow from array elements of the base of a `SliceNode` to
30+
// the `SliceNode` itself, we consider there to be a read step with array
31+
// content from the base to the corresponding `SliceElementNode` and then
32+
// a store step with array content from the `SliceelementNode` to the
33+
// `SliceNode` itself.
34+
node2 = node1.(SliceElementNode).getSliceNode()
35+
)
3436
)
37+
or
38+
c instanceof CollectionContent and
39+
exists(SendStmt send |
40+
send.getChannel() = node2.(ExprNode).asExpr() and send.getValue() = node1.(ExprNode).asExpr()
41+
)
42+
or
43+
c instanceof MapKeyContent and
44+
t instanceof MapType and
45+
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), node1, _))
46+
or
47+
c instanceof MapValueContent and
48+
t instanceof MapType and
49+
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
3550
)
36-
or
37-
c instanceof CollectionContent and
38-
exists(SendStmt send |
39-
send.getChannel() = node2.(ExprNode).asExpr() and send.getValue() = node1.(ExprNode).asExpr()
40-
)
41-
or
42-
c instanceof MapKeyContent and
43-
node2.getType() instanceof MapType and
44-
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), node1, _))
45-
or
46-
c instanceof MapValueContent and
47-
node2.getType() instanceof MapType and
48-
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
4951
}
5052

5153
/**
@@ -55,35 +57,37 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
5557
* as well as array iteration through enhanced `for` statements.
5658
*/
5759
predicate containerReadStep(Node node1, Node node2, Content c) {
58-
c instanceof ArrayContent and
59-
(
60-
node1.getType() instanceof ArrayType or
61-
node1.getType() instanceof SliceType
62-
) and
63-
(
64-
node2.(Read).readsElement(node1, _)
60+
exists(Type t | t = node1.getType().getUnderlyingType() |
61+
c instanceof ArrayContent and
62+
(
63+
t instanceof ArrayType or
64+
t instanceof SliceType
65+
) and
66+
(
67+
node2.(Read).readsElement(node1, _)
68+
or
69+
node2.(RangeElementNode).getBase() = node1
70+
or
71+
// To model data flow from array elements of the base of a `SliceNode` to
72+
// the `SliceNode` itself, we consider there to be a read step with array
73+
// content from the base to the corresponding `SliceElementNode` and then
74+
// a store step with array content from the `SliceelementNode` to the
75+
// `SliceNode` itself.
76+
node2.(SliceElementNode).getSliceNode().getBase() = node1
77+
)
6578
or
66-
node2.(RangeElementNode).getBase() = node1
79+
c instanceof CollectionContent and
80+
exists(UnaryOperationNode recv | recv = node2 |
81+
node1 = recv.getOperand() and
82+
recv.getOperator() = "<-"
83+
)
6784
or
68-
// To model data flow from array elements of the base of a `SliceNode` to
69-
// the `SliceNode` itself, we consider there to be a read step with array
70-
// content from the base to the corresponding `SliceElementNode` and then
71-
// a store step with array content from the `SliceelementNode` to the
72-
// `SliceNode` itself.
73-
node2.(SliceElementNode).getSliceNode().getBase() = node1
74-
)
75-
or
76-
c instanceof CollectionContent and
77-
exists(UnaryOperationNode recv | recv = node2 |
78-
node1 = recv.getOperand() and
79-
recv.getOperator() = "<-"
85+
c instanceof MapKeyContent and
86+
t instanceof MapType and
87+
node2.(RangeIndexNode).getBase() = node1
88+
or
89+
c instanceof MapValueContent and
90+
t instanceof MapType and
91+
(node2.(Read).readsElement(node1, _) or node2.(RangeElementNode).getBase() = node1)
8092
)
81-
or
82-
c instanceof MapKeyContent and
83-
node1.getType() instanceof MapType and
84-
node2.(RangeIndexNode).getBase() = node1
85-
or
86-
c instanceof MapValueContent and
87-
node1.getType() instanceof MapType and
88-
(node2.(Read).readsElement(node1, _) or node2.(RangeElementNode).getBase() = node1)
8993
}

‎go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ func simpleflow() {
117117
b.Sink1(k) // $ hasTaintFlow="k"
118118
}
119119
for k, _ := range mapstringstringtype(taint12) {
120-
b.Sink1(k) // $ MISSING: hasTaintFlow="k"
120+
b.Sink1(k) // $ hasTaintFlow="k"
121121
}
122122
for k := range mapstringstringtype(taint12) {
123-
b.Sink1(k) // $ MISSING: hasTaintFlow="k"
123+
b.Sink1(k) // $ hasTaintFlow="k"
124124
}
125125

126126
srcMap13 := map[string]string{src.(string): ""}
@@ -133,7 +133,7 @@ func simpleflow() {
133133
b.Sink1(v) // $ hasTaintFlow="v"
134134
}
135135
for _, v := range mapstringstringtype(taint14) {
136-
b.Sink1(v) // $ MISSING: hasTaintFlow="v"
136+
b.Sink1(v) // $ hasTaintFlow="v"
137137
}
138138

139139
srcMap15 := map[string]string{"": src.(string)}

‎go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func simpleflow() {
8787
b.Sink1(x) // $ hasValueFlow="x"
8888
}
8989
for _, x := range arraytype(taint8) {
90-
b.Sink1(x) // $ MISSING: hasValueFlow="x"
90+
b.Sink1(x) // $ hasValueFlow="x"
9191
}
9292

9393
srcArray := []interface{}{nil, src}
@@ -117,10 +117,10 @@ func simpleflow() {
117117
b.Sink1(k) // $ hasValueFlow="k"
118118
}
119119
for k, _ := range mapstringstringtype(taint12) {
120-
b.Sink1(k) // $ MISSING: hasValueFlow="k"
120+
b.Sink1(k) // $ hasValueFlow="k"
121121
}
122122
for k := range mapstringstringtype(taint12) {
123-
b.Sink1(k) // $ MISSING: hasValueFlow="k"
123+
b.Sink1(k) // $ hasValueFlow="k"
124124
}
125125

126126
srcMap13 := map[string]string{src.(string): ""}
@@ -133,7 +133,7 @@ func simpleflow() {
133133
b.Sink1(v) // $ hasValueFlow="v"
134134
}
135135
for _, v := range mapstringstringtype(taint14) {
136-
b.Sink1(v) // $ MISSING: hasValueFlow="v"
136+
b.Sink1(v) // $ hasValueFlow="v"
137137
}
138138

139139
srcMap15 := map[string]string{"": src.(string)}

0 commit comments

Comments
 (0)