Skip to content

Commit f89f3d3

Browse files
owen-mcowen-mc
committed
Add and use WriteNode.writesFieldPreUpdate 
1 parent e7cbca0 commit f89f3d3

File tree

5 files changed

+40
-72
lines changed

5 files changed

+40
-72
lines changed

‎go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,31 +134,42 @@ module ControlFlow {
134134

135135
/**
136136
* Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
137-
* `rhs`.
137+
* `rhs`, where `base` represents the post-update value.
138138
*
139139
* For example, for the assignment `x.width = newWidth`, `base` is the post-update node of
140140
* either the data-flow node corresponding to `x` or (if `x` is a pointer) the data-flow node
141141
* corresponding to the implicit dereference `*x`, `f` is the field referenced by `width`, and
142142
* `rhs` is the data-flow node corresponding to `newWidth`. If this `WriteNode` is a struct
143-
* initialization then there is no need for a post-update node and `base` is the struct literal
144-
* being initialized.
143+
* initialization then there is no post-update node and `base` is the struct literal being
144+
* initialized.
145145
*/
146146
predicate writesField(DataFlow::Node base, Field f, DataFlow::Node rhs) {
147-
exists(DataFlow::Node b | this.writesFieldInsn(b.asInstruction(), f, rhs.asInstruction()) |
147+
exists(DataFlow::Node b | this.writesFieldPreUpdate(b, f, rhs) |
148148
this.isInitialization() and base = b
149149
or
150150
not this.isInitialization() and
151151
b = base.(DataFlow::PostUpdateNode).getPreUpdateNode()
152152
)
153153
}
154154

155+
/**
156+
* Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
157+
* `rhs`, where `base` represents the pre-update value.
158+
*
159+
* For example, for the assignment `x.width = newWidth`, `base` is either the data-flow node
160+
* corresponding to `x` or (if `x` is a pointer) the data-flow node corresponding to the
161+
* implicit dereference `*x`, `f` is the field referenced by `width`, and `rhs` is the
162+
* data-flow node corresponding to `newWidth`.
163+
*/
164+
predicate writesFieldPreUpdate(DataFlow::Node base, Field f, DataFlow::Node rhs) {
165+
this.writesFieldInsn(base.asInstruction(), f, rhs.asInstruction())
166+
}
167+
155168
/**
156169
* Holds if this node sets the value of field `f` on `v` to `rhs`.
157170
*/
158171
predicate writesFieldOnSsaWithFields(SsaWithFields v, Field f, DataFlow::Node rhs) {
159-
exists(IR::Instruction insn | this.writesFieldInsn(insn, f, rhs.asInstruction()) |
160-
v.getAUse().asInstruction() = insn
161-
)
172+
this.writesFieldPreUpdate(v.getAUse(), f, rhs)
162173
}
163174

164175
private predicate writesFieldInsn(IR::Instruction base, Field f, IR::Instruction rhs) {

‎go/ql/lib/semmle/go/frameworks/GinCors.qll‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,10 @@ module GinCors {
2525
DataFlow::Node base;
2626

2727
AllowCredentialsWrite() {
28-
exists(Field f, Write w, DataFlow::Node n |
28+
exists(Field f, Write w |
2929
f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
30-
w.writesField(n, f, this) and
31-
this.getType() instanceof BoolType and
32-
(
33-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
34-
or
35-
not n instanceof DataFlow::PostUpdateNode and base = n
36-
)
30+
w.writesFieldPreUpdate(base, f, this) and
31+
this.getType() instanceof BoolType
3732
)
3833
}
3934

@@ -64,15 +59,10 @@ module GinCors {
6459
DataFlow::Node base;
6560

6661
AllowOriginsWrite() {
67-
exists(Field f, Write w, DataFlow::Node n |
62+
exists(Field f, Write w |
6863
f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
69-
w.writesField(n, f, this) and
70-
this.asExpr() instanceof SliceLit and
71-
(
72-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
73-
or
74-
not n instanceof DataFlow::PostUpdateNode and base = n
75-
)
64+
w.writesFieldPreUpdate(base, f, this) and
65+
this.asExpr() instanceof SliceLit
7666
)
7767
}
7868

@@ -103,15 +93,10 @@ module GinCors {
10393
DataFlow::Node base;
10494

10595
AllowAllOriginsWrite() {
106-
exists(Field f, Write w, DataFlow::Node n |
96+
exists(Field f, Write w |
10797
f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
108-
w.writesField(n, f, this) and
109-
this.getType() instanceof BoolType and
110-
(
111-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
112-
or
113-
not n instanceof DataFlow::PostUpdateNode and base = n
114-
)
98+
w.writesFieldPreUpdate(base, f, this) and
99+
this.getType() instanceof BoolType
115100
)
116101
}
117102

‎go/ql/lib/semmle/go/frameworks/RsCors.qll‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,10 @@ module RsCors {
5252
DataFlow::Node base;
5353

5454
AllowCredentialsWrite() {
55-
exists(Field f, Write w, DataFlow::Node n |
55+
exists(Field f, Write w |
5656
f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and
57-
w.writesField(n, f, this) and
58-
this.getType() instanceof BoolType and
59-
(
60-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
61-
or
62-
not n instanceof DataFlow::PostUpdateNode and base = n
63-
)
57+
w.writesFieldPreUpdate(base, f, this) and
58+
this.getType() instanceof BoolType
6459
)
6560
}
6661

@@ -85,15 +80,10 @@ module RsCors {
8580
DataFlow::Node base;
8681

8782
AllowOriginsWrite() {
88-
exists(Field f, Write w, DataFlow::Node n |
83+
exists(Field f, Write w |
8984
f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and
90-
w.writesField(n, f, this) and
91-
this.asExpr() instanceof SliceLit and
92-
(
93-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
94-
or
95-
not n instanceof DataFlow::PostUpdateNode and base = n
96-
)
85+
w.writesFieldPreUpdate(base, f, this) and
86+
this.asExpr() instanceof SliceLit
9787
)
9888
}
9989

@@ -121,15 +111,10 @@ module RsCors {
121111
DataFlow::Node base;
122112

123113
AllowAllOriginsWrite() {
124-
exists(Field f, Write w, DataFlow::Node n |
114+
exists(Field f, Write w |
125115
f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and
126-
w.writesField(n, f, this) and
127-
this.getType() instanceof BoolType and
128-
(
129-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
130-
or
131-
not n instanceof DataFlow::PostUpdateNode and base = n
132-
)
116+
w.writesFieldPreUpdate(base, f, this) and
117+
this.getType() instanceof BoolType
133118
)
134119
}
135120

‎go/ql/src/Security/CWE-327/InsecureTLS.ql‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,7 @@ module TlsVersionFlowConfig implements DataFlow::ConfigSig {
6565
*/
6666
additional predicate isSink(DataFlow::Node sink, Field fld, DataFlow::Node base, Write fieldWrite) {
6767
fld.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) and
68-
exists(DataFlow::Node n | fieldWrite.writesField(n, fld, sink) |
69-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
70-
or
71-
not n instanceof DataFlow::PostUpdateNode and base = n
72-
)
68+
fieldWrite.writesFieldPreUpdate(base, fld, sink)
7369
}
7470

7571
predicate isSource(DataFlow::Node source) { intIsSource(source, _) }
@@ -194,11 +190,7 @@ module TlsInsecureCipherSuitesFlowConfig implements DataFlow::ConfigSig {
194190
*/
195191
additional predicate isSink(DataFlow::Node sink, Field fld, DataFlow::Node base, Write fieldWrite) {
196192
fld.hasQualifiedName("crypto/tls", "Config", "CipherSuites") and
197-
exists(DataFlow::Node n | fieldWrite.writesField(n, fld, sink) |
198-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
199-
or
200-
not n instanceof DataFlow::PostUpdateNode and base = n
201-
)
193+
fieldWrite.writesFieldPreUpdate(base, fld, sink)
202194
}
203195

204196
predicate isSink(DataFlow::Node sink) { isSink(sink, _, _, _) }

‎go/ql/src/experimental/CWE-1004/AuthCookie.qll‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,9 @@ private class GorillaSessionOptionsField extends Field {
2626
* This should cover most typical patterns...
2727
*/
2828
private DataFlow::Node getValueForFieldWrite(StructLit sl, string field) {
29-
exists(Write w, DataFlow::Node base, DataFlow::Node n, Field f |
29+
exists(Write w, DataFlow::Node base, Field f |
3030
f.getName() = field and
31-
w.writesField(n, f, result) and
32-
(
33-
base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
34-
or
35-
not n instanceof DataFlow::PostUpdateNode and base = n
36-
) and
31+
w.writesFieldPreUpdate(base, f, result) and
3732
(
3833
sl = base.asExpr()
3934
or

0 commit comments

Comments
 (0)