Add and use WriteNode.writesFieldPreUpdate

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

@@ -134,17 +134,17 @@ module ControlFlow {
 
     /**
      * Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
-     * `rhs`.
+     * `rhs`, where `base` represents the post-update value.
      *
      * For example, for the assignment `x.width = newWidth`, `base` is the post-update node of
      * either the data-flow node corresponding to `x` or (if `x` is a pointer) the data-flow node
      * corresponding to the implicit dereference `*x`, `f` is the field referenced by `width`, and
      * `rhs` is the data-flow node corresponding to `newWidth`. If this `WriteNode` is a struct
-     * initialization then there is no need for a post-update node and `base` is the struct literal
-     * being initialized.
+     * initialization then there is no post-update node and `base` is the struct literal being
+     * initialized.
      */
     predicate writesField(DataFlow::Node base, Field f, DataFlow::Node rhs) {
-      exists(DataFlow::Node b | this.writesFieldInsn(b.asInstruction(), f, rhs.asInstruction()) |
+      exists(DataFlow::Node b | this.writesFieldPreUpdate(b, f, rhs) |
         this.isInitialization() and base = b
         or
         not this.isInitialization() and
@@ -152,13 +152,24 @@ module ControlFlow {
       )
     }
 
+    /**
+     * Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
+     * `rhs`, where `base` represents the pre-update value.
+     *
+     * For example, for the assignment `x.width = newWidth`, `base` is either the data-flow node
+     * corresponding to `x` or (if `x` is a pointer) the data-flow node corresponding to the
+     * implicit dereference `*x`, `f` is the field referenced by `width`, and `rhs` is the
+     * data-flow node corresponding to `newWidth`.
+     */
+    predicate writesFieldPreUpdate(DataFlow::Node base, Field f, DataFlow::Node rhs) {
+      this.writesFieldInsn(base.asInstruction(), f, rhs.asInstruction())
+    }
+
     /**
      * Holds if this node sets the value of field `f` on `v` to `rhs`.
      */
     predicate writesFieldOnSsaWithFields(SsaWithFields v, Field f, DataFlow::Node rhs) {
-      exists(IR::Instruction insn | this.writesFieldInsn(insn, f, rhs.asInstruction()) |
-        v.getAUse().asInstruction() = insn
-      )
+      this.writesFieldPreUpdate(v.getAUse(), f, rhs)
     }
 
     private predicate writesFieldInsn(IR::Instruction base, Field f, IR::Instruction rhs) {

go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

@@ -437,20 +437,13 @@ module SourceSinkInterpretationInput implements
       mid.asCallable() = getNodeEnclosingCallable(ret)
     )
     or
-    exists(
-      SourceOrSinkElement e, DataFlow::Write fw, DataFlow::Node base, DataFlow::Node qual, Field f
-    |
+    exists(SourceOrSinkElement e, DataFlow::Write fw, DataFlow::Node base, Field f |
       e = mid.asElement() and
       f = e.asFieldEntity()
     |
       c = "" and
-      fw.writesField(base, f, node.asNode()) and
-      pragma[only_bind_into](e) = getElementWithQualifier(f, qual) and
-      (
-        qual = base.(PostUpdateNode).getPreUpdateNode()
-        or
-        not base instanceof PostUpdateNode and qual = base
-      )
+      fw.writesFieldPreUpdate(base, f, node.asNode()) and
+      pragma[only_bind_into](e) = getElementWithQualifier(f, base)
     )
     or
     // A package-scope (or universe-scope) variable

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

@@ -25,15 +25,10 @@ module GinCors {
     DataFlow::Node base;
 
     AllowCredentialsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
-        w.writesField(n, f, this) and
-        this.getType() instanceof BoolType and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.getType() instanceof BoolType
       )
     }
 
@@ -64,15 +59,10 @@ module GinCors {
     DataFlow::Node base;
 
     AllowOriginsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
-        w.writesField(n, f, this) and
-        this.asExpr() instanceof SliceLit and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.asExpr() instanceof SliceLit
       )
     }
 
@@ -103,15 +93,10 @@ module GinCors {
     DataFlow::Node base;
 
     AllowAllOriginsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
-        w.writesField(n, f, this) and
-        this.getType() instanceof BoolType and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.getType() instanceof BoolType
       )
     }
 

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

@@ -52,15 +52,10 @@ module RsCors {
     DataFlow::Node base;
 
     AllowCredentialsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and
-        w.writesField(n, f, this) and
-        this.getType() instanceof BoolType and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.getType() instanceof BoolType
       )
     }
 
@@ -85,15 +80,10 @@ module RsCors {
     DataFlow::Node base;
 
     AllowOriginsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and
-        w.writesField(n, f, this) and
-        this.asExpr() instanceof SliceLit and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.asExpr() instanceof SliceLit
       )
     }
 
@@ -121,15 +111,10 @@ module RsCors {
     DataFlow::Node base;
 
     AllowAllOriginsWrite() {
-      exists(Field f, Write w, DataFlow::Node n |
+      exists(Field f, Write w |
         f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and
-        w.writesField(n, f, this) and
-        this.getType() instanceof BoolType and
-        (
-          base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-          or
-          not n instanceof DataFlow::PostUpdateNode and base = n
-        )
+        w.writesFieldPreUpdate(base, f, this) and
+        this.getType() instanceof BoolType
       )
     }
 

go/ql/src/RedundantCode/DeadStoreOfField.ql

@@ -86,11 +86,10 @@ Type getTypeEmbeddedViaPointer(Type t) {
   result = getEmbeddedType*(getEmbeddedType(getEmbeddedType*(t), true))
 }
 
-from Write w, DataFlow::Node base, LocalVariable v, Field f
+from Write w, LocalVariable v, Field f
 where
   // `w` writes `f` on `v`
-  w.writesField(base, f, _) and
-  [base, base.(DataFlow::PostUpdateNode).getPreUpdateNode()] = v.getARead() and
+  w.writesFieldPreUpdate(v.getARead(), f, _) and
   // but `f` is never read on `v`
   not exists(Read r | r.readsField(v.getARead(), f)) and
   // exclude pointer-typed `v`; there may be reads through an alias

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

@@ -65,11 +65,7 @@ module TlsVersionFlowConfig implements DataFlow::ConfigSig {
    */
   additional predicate isSink(DataFlow::Node sink, Field fld, DataFlow::Node base, Write fieldWrite) {
     fld.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) and
-    exists(DataFlow::Node n | fieldWrite.writesField(n, fld, sink) |
-      base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-      or
-      not n instanceof DataFlow::PostUpdateNode and base = n
-    )
+    fieldWrite.writesFieldPreUpdate(base, fld, sink)
   }
 
   predicate isSource(DataFlow::Node source) { intIsSource(source, _) }
@@ -194,11 +190,7 @@ module TlsInsecureCipherSuitesFlowConfig implements DataFlow::ConfigSig {
    */
   additional predicate isSink(DataFlow::Node sink, Field fld, DataFlow::Node base, Write fieldWrite) {
     fld.hasQualifiedName("crypto/tls", "Config", "CipherSuites") and
-    exists(DataFlow::Node n | fieldWrite.writesField(n, fld, sink) |
-      base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-      or
-      not n instanceof DataFlow::PostUpdateNode and base = n
-    )
+    fieldWrite.writesFieldPreUpdate(base, fld, sink)
   }
 
   predicate isSink(DataFlow::Node sink) { isSink(sink, _, _, _) }

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

@@ -26,14 +26,9 @@ private class GorillaSessionOptionsField extends Field {
  * This should cover most typical patterns...
  */
 private DataFlow::Node getValueForFieldWrite(StructLit sl, string field) {
-  exists(Write w, DataFlow::Node base, DataFlow::Node n, Field f |
+  exists(Write w, DataFlow::Node base, Field f |
     f.getName() = field and
-    w.writesField(n, f, result) and
-    (
-      base = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
-      or
-      not n instanceof DataFlow::PostUpdateNode and base = n
-    ) and
+    w.writesFieldPreUpdate(base, f, result) and
     (
       sl = base.asExpr()
       or