diff --git a/go/src/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.ql b/go/src/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.ql index 81b736f..2e8314b 100644 --- a/go/src/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.ql +++ b/go/src/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.ql @@ -11,8 +11,6 @@ */ import go -import DataFlow::PathGraph -import semmle.go.dataflow.DataFlow2 /** * Function that performs signing or signature verification on a hash of a message @@ -90,27 +88,25 @@ class HashFunction extends Function { } } -class LongestFlowConfig extends DataFlow2::Configuration { - LongestFlowConfig() { this = "LongestFlowConfig" } - override predicate isSource(DataFlow::Node source) { source = source } - override predicate isSink(DataFlow::Node sink) { sink = sink } +private module LongestFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source = source } + predicate isSink(DataFlow::Node sink) { sink = sink } } +module LongestFlowFlow = DataFlow::Global; /** * Flows from anything to SignatureMsgTruncationFunction * that do not cross a hash function or slicing expression */ -class AnythingToSignatureMsgTrunFuncFlow extends DataFlow::Configuration { - AnythingToSignatureMsgTrunFuncFlow() { this = "AnythingToSignatureMsgTrunFuncFlow" } - +module AnythingToSignatureMsgTrunFuncConfig implements DataFlow::ConfigSig { // anything that is not a function's argument // TODO: alternatively, set sources to be ExternalInputs - override predicate isSource(DataFlow::Node source) { - not this.isSink(source, _) + predicate isSource(DataFlow::Node source) { + not isSink(source) and not source.asInstruction() instanceof IR::ReadArgumentInstruction } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(SignatureMsgTruncationFunction sigUseF, CallExpr sigUseCall, int position | sigUseCall.getTarget() = sigUseF and sigUseF.hashArgPosition(position) @@ -122,7 +118,7 @@ class AnythingToSignatureMsgTrunFuncFlow extends DataFlow::Configuration { // * data goes through a hash function // * data is truncated with a hardcoded value // * TODO: data is of type Hash - override predicate isBarrier(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { // direct hash function call exists(HashFunction hf | hf.getACall().getResult(_) = node or hf.getACall().getArgument(_) = node) or @@ -142,14 +138,17 @@ class AnythingToSignatureMsgTrunFuncFlow extends DataFlow::Configuration { node.asExpr().getType().getUnderlyingType().(ArrayType).getLength() <= 66 } } +module AnythingToSignatureMsgTrunFuncFlow = DataFlow::Global; +import AnythingToSignatureMsgTrunFuncFlow::PathGraph -from AnythingToSignatureMsgTrunFuncFlow config, DataFlow::PathNode source, DataFlow::PathNode sink +from AnythingToSignatureMsgTrunFuncFlow::PathNode source, AnythingToSignatureMsgTrunFuncFlow::PathNode sink where - config.hasFlowPath(source, sink) + AnythingToSignatureMsgTrunFuncFlow::flowPath(source, sink) // only the longest flow - and not exists(LongestFlowConfig config2, DataFlow::Node source2 | - config2.hasFlow(source2, source.getNode()) + // TODO: find only flows originating from user input + and not exists(DataFlow::Node source2 | + LongestFlowFlow::flow(source2, source.getNode()) and source2 != source.getNode() ) diff --git a/go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql b/go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql index c70abf0..b79bb8b 100644 --- a/go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql +++ b/go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql @@ -15,13 +15,11 @@ import go /** * Flow of a `tls.Config` to a write to the `MinVersion` field. */ -class TlsVersionFlowConfig extends TaintTracking::Configuration { - TlsVersionFlowConfig() { this = "TlsVersionFlowConfig" } - +module TlsVersionConfig implements DataFlow::ConfigSig { /** * Holds if `source` is a TLS.Config instance. */ - override predicate isSource(DataFlow::Node source) { + predicate isSource(DataFlow::Node source) { exists(Variable v | configOrConfigPointer(v.getType()) and source.asExpr() = v.getAReference() @@ -31,21 +29,21 @@ class TlsVersionFlowConfig extends TaintTracking::Configuration { /** * Holds if a write to `sink`.MinVersion exists. */ - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(Write fieldWrite, Field fld | fld.hasQualifiedName( "crypto/tls", "Config", "MinVersion") and fieldWrite.writesField(sink, fld, _) ) } } +module TlsVersionFlow = TaintTracking::Global; + /** * Flow of a `tls.Config` with `MinVersion` to a variable. */ -class TlsConfigCreation extends TaintTracking::Configuration { - TlsConfigCreation() { this = "TlsConfigCreation" } - - predicate isSecure(DataFlow::Node source) { +module TlsConfigCreationConfig implements DataFlow::ConfigSig { + additional predicate isSecure(DataFlow::Node source) { exists(StructLit lit, Field fld | lit.getType().hasQualifiedName("crypto/tls", "Config") and fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and @@ -58,18 +56,19 @@ class TlsConfigCreation extends TaintTracking::Configuration { /** * Holds if `source` is a TLS.Config literal. */ - override predicate isSource(DataFlow::Node source) { + predicate isSource(DataFlow::Node source) { exists(StructLit lit, Field fld | lit.getType().hasQualifiedName("crypto/tls", "Config") and fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and source.asExpr() = lit ) + and not isSecure(source) } /** * Holds if it is TLS.Config instance (a Variable). */ - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(Variable v | sink.asExpr() = v.getAReference() ) @@ -78,10 +77,11 @@ class TlsConfigCreation extends TaintTracking::Configuration { /** * Holds if TLS.Config literal is saved in a structure's field */ - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { exists(Write w | w.writesField(succ, _, pred)) } } +module TlsConfigCreationFlow = TaintTracking::Global; /** * Holds if `t` is a TLS.Config type or a pointer to it (or ptr to ptr...) or a struct containing it. @@ -104,14 +104,13 @@ predicate configOrConfigPointer(Type t) { } // v - a variable holding any structure which is or contains the tls.Config -from StructLit configStruct, Variable v, TlsConfigCreation cfg, DataFlow::Node source, DataFlow::Node sink +from StructLit configStruct, Variable v, DataFlow::Node source, DataFlow::Node sink where // find tls.Config structures with MinVersion not set on the structure initialization ( - cfg.hasFlow(source, sink) and + TlsConfigCreationFlow::flow(source, sink) and sink.asExpr() = v.getAReference() and - source.asExpr() = configStruct and - not cfg.isSecure(source) + source.asExpr() = configStruct ) // exclude if tls.Config is used as TLSClientConfig, as default for clients is TLS 1.2 @@ -143,8 +142,8 @@ where and if configOrConfigPointer(v.getType()) then ( // exclude if there is a later write to MinVersion - not exists(TlsVersionFlowConfig cfg2, DataFlow::Node source2, DataFlow::Node sink2 | - cfg2.hasFlow(source2, sink2) and + not exists(DataFlow::Node source2, DataFlow::Node sink2 | + TlsVersionFlow::flow(source2, sink2) and source2.asExpr() = v.getAReference() ) ) else diff --git a/go/src/security/TrimMisuse/TrimMisuse.ql b/go/src/security/TrimMisuse/TrimMisuse.ql index 746598e..311e1a0 100644 --- a/go/src/security/TrimMisuse/TrimMisuse.ql +++ b/go/src/security/TrimMisuse/TrimMisuse.ql @@ -11,24 +11,23 @@ */ import go -import DataFlow +import DataFlow2 /* * Flows from a string to TrimFamilyCall cutSet argument */ -class Trim2ndArg extends DataFlow::Configuration { - Trim2ndArg() { this = "Trim2ndArg" } - - override predicate isSource(DataFlow::Node source) { +module Trim2ndArgConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof StringLit } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(TrimFamilyCall trimCall | sink.asExpr() = trimCall.getCutSetArg() ) } } +module Trim2ndArgFlow = DataFlow::Global; /* * Calls to Trim methods that we are interested in @@ -49,8 +48,8 @@ class TrimFamilyCall extends CallNode { from TrimFamilyCall trimCall, StringLit cutset where // get 2nd argument value, if possible - exists(Trim2ndArg config, DataFlow::Node source, DataFlow::Node sink | - config.hasFlow(source, sink) + exists(DataFlow::Node source, DataFlow::Node sink | + Trim2ndArgFlow::flow(source, sink) and source.asExpr() = cutset and sink.asExpr() = trimCall.getCutSetArg() ) diff --git a/go/test/query-tests/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.expected b/go/test/query-tests/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.expected index 4c7f0ab..b2e933d 100644 --- a/go/test/query-tests/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.expected +++ b/go/test/query-tests/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.expected @@ -1,6 +1,6 @@ edges -| MsgNotHashedBeforeSigVerfication.go:78:33:78:46 | "test message" | MsgNotHashedBeforeSigVerfication.go:78:26:78:47 | type conversion | -| MsgNotHashedBeforeSigVerfication.go:86:31:86:44 | "test message" | MsgNotHashedBeforeSigVerfication.go:86:24:86:45 | type conversion | +| MsgNotHashedBeforeSigVerfication.go:78:33:78:46 | "test message" | MsgNotHashedBeforeSigVerfication.go:78:26:78:47 | type conversion | provenance | | +| MsgNotHashedBeforeSigVerfication.go:86:31:86:44 | "test message" | MsgNotHashedBeforeSigVerfication.go:86:24:86:45 | type conversion | provenance | | nodes | MsgNotHashedBeforeSigVerfication.go:78:26:78:47 | type conversion | semmle.label | type conversion | | MsgNotHashedBeforeSigVerfication.go:78:33:78:46 | "test message" | semmle.label | "test message" |