diff --git a/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll b/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll index 7d38fbe934d..e66feeadaac 100644 --- a/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll +++ b/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) { diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index a7dfbab15c2..f12c9e6eeb1 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/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 diff --git a/go/ql/lib/semmle/go/frameworks/GinCors.qll b/go/ql/lib/semmle/go/frameworks/GinCors.qll index a26ab2eaa12..cc993ea4dee 100644 --- a/go/ql/lib/semmle/go/frameworks/GinCors.qll +++ b/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 ) } diff --git a/go/ql/lib/semmle/go/frameworks/RsCors.qll b/go/ql/lib/semmle/go/frameworks/RsCors.qll index aa49df3c432..52b4a7fe6d0 100644 --- a/go/ql/lib/semmle/go/frameworks/RsCors.qll +++ b/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 ) } diff --git a/go/ql/src/RedundantCode/DeadStoreOfField.ql b/go/ql/src/RedundantCode/DeadStoreOfField.ql index 3f757cd8b54..4a971343823 100644 --- a/go/ql/src/RedundantCode/DeadStoreOfField.ql +++ b/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 diff --git a/go/ql/src/Security/CWE-327/InsecureTLS.ql b/go/ql/src/Security/CWE-327/InsecureTLS.ql index 66e516a85eb..b5d8a81f3d8 100644 --- a/go/ql/src/Security/CWE-327/InsecureTLS.ql +++ b/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, _, _, _) } diff --git a/go/ql/src/experimental/CWE-1004/AuthCookie.qll b/go/ql/src/experimental/CWE-1004/AuthCookie.qll index 2cf8577ac5a..58c9f8642b3 100644 --- a/go/ql/src/experimental/CWE-1004/AuthCookie.qll +++ b/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