Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go fix dataflows new #9

Merged
merged 4 commits into from
Jul 8, 2024
Merged

Go fix dataflows new #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40423da
start fixing in MsgNotHashedBeforeSigVerfication
GrosQuildu Jul 5, 2024
a5825c9
MsgNotHashedBeforeSigVerfication fixed
GrosQuildu Jul 5, 2024
3737840
fix mintlsversion
GrosQuildu Jul 5, 2024
37062f5
fix trim misuse
GrosQuildu Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions go/src/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<LongestFlowConfig>;

/**
* 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)
Expand All @@ -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
Expand All @@ -142,14 +138,17 @@ class AnythingToSignatureMsgTrunFuncFlow extends DataFlow::Configuration {
node.asExpr().getType().getUnderlyingType().(ArrayType).getLength() <= 66
}
}
module AnythingToSignatureMsgTrunFuncFlow = DataFlow::Global<AnythingToSignatureMsgTrunFuncConfig>;
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()
)

Expand Down
35 changes: 17 additions & 18 deletions go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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<TlsVersionConfig>;


/**
* 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
Expand All @@ -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()
)
Expand All @@ -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<TlsConfigCreationConfig>;

/**
* Holds if `t` is a TLS.Config type or a pointer to it (or ptr to ptr...) or a struct containing it.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions go/src/security/TrimMisuse/TrimMisuse.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Trim2ndArgConfig>;

/*
* Calls to Trim methods that we are interested in
Expand All @@ -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()
)
Expand Down
4 changes: 2 additions & 2 deletions ...y-tests/crypto/MsgNotHashedBeforeSigVerfication/MsgNotHashedBeforeSigVerfication.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 | |
GrosQuildu marked this conversation as resolved.
Show resolved Hide resolved
| 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" |
Expand Down
Loading