From bbf8d7306b2614fda0c73f07d7dd8cdcc1df7045 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 16:54:14 +0300 Subject: [PATCH 01/11] Add CWE-327 --- ql/src/experimental/CWE-327/InsecureTLS.go | 31 +++ ql/src/experimental/CWE-327/InsecureTLS.qhelp | 47 ++++ ql/src/experimental/CWE-327/InsecureTLS.ql | 160 +++++++++++++ ql/src/experimental/CWE-327/SaferTLS.go | 11 + .../experimental/CWE-327/UnsafeTLS.expected | 100 ++++++++ ql/test/experimental/CWE-327/UnsafeTLS.go | 214 ++++++++++++++++++ ql/test/experimental/CWE-327/UnsafeTLS.qlref | 1 + 7 files changed, 564 insertions(+) create mode 100644 ql/src/experimental/CWE-327/InsecureTLS.go create mode 100644 ql/src/experimental/CWE-327/InsecureTLS.qhelp create mode 100644 ql/src/experimental/CWE-327/InsecureTLS.ql create mode 100644 ql/src/experimental/CWE-327/SaferTLS.go create mode 100644 ql/test/experimental/CWE-327/UnsafeTLS.expected create mode 100644 ql/test/experimental/CWE-327/UnsafeTLS.go create mode 100644 ql/test/experimental/CWE-327/UnsafeTLS.qlref diff --git a/ql/src/experimental/CWE-327/InsecureTLS.go b/ql/src/experimental/CWE-327/InsecureTLS.go new file mode 100644 index 00000000000..4d78dbc2aa9 --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureTLS.go @@ -0,0 +1,31 @@ +package main + +import ( + "crypto/tls" +) + +func main() {} + +func insecureMinMaxTlsVersion() { + { + config := &tls.Config{} + config.MinVersion = 0 //BAD: Setting the MinVersion to 0 equal to choosing the lowest supported version (i.e. SSL3.0) + } + { + config := &tls.Config{} + config.MinVersion = tls.VersionSSL30 //BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MinVersion. + } + { + config := &tls.Config{} + config.MaxVersion = tls.VersionSSL30 //BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MaxVersion. + } +} + +func insecureCipherSuites() { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_RSA_WITH_RC4_128_SHA, //BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure chiper suites; it's not safe to be used. + }, + } + _ = config +} diff --git a/ql/src/experimental/CWE-327/InsecureTLS.qhelp b/ql/src/experimental/CWE-327/InsecureTLS.qhelp new file mode 100644 index 00000000000..fcdb3d8875b --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureTLS.qhelp @@ -0,0 +1,47 @@ + + + +

+ The TLS (Transport Layer Security) protocol secures communications over the Internet. + The protocol allows client/server applications to communicate in a way that is designed + to prevent eavesdropping, tampering, or message forgery. +

+

+ The current latest version is 1.3 (with the 1.2 version still being considered secure). + Older versions are not deemed to be secure anymore because of various security vulnerabilities, + and tht makes them unfit for use in securing your applications. +

+

+ Unfortunately, many applications and websites still support deprecated SSL/TLS versions and + cipher suites. +

+
+ +

+ Only use secure TLS versions (1.3 and 1.2) and avoid using insecure cipher suites + (you can see a list here: https://golang.org/src/crypto/tls/cipher_suites.go#L81) +

+
+ +

+ The following example shows a few ways how an insecure TLS configuration can be created: +

+ +

+ The following example shows how to create a safer TLS configuration: +

+ +
+ +
  • + Wikipedia: + Transport Layer Security +
  • +
  • + OWASP: + Transport Layer Protection Cheat Sheet +
  • +
    +
    \ No newline at end of file diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql new file mode 100644 index 00000000000..0f7af28e035 --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -0,0 +1,160 @@ +/** + * @name Insecure TLS configuration + * @description If an application supports insecure TLS versions or ciphers, it may be vulnerable to + * man-in-the-middle and other attacks. + * @kind path-problem + * @problem.severity warning + * @id go/insecure-tls + * @tags security + * external/cwe/cwe-327 + */ + +import go +import DataFlow::PathGraph + +/** + * Check whether the file where the node is located is a test file. + */ +predicate isTestFile(DataFlow::Node node) { + // Exclude results in test files: + exists(File fl | fl = node.getRoot().getFile() | + fl instanceof TestFile or fl.getPackageName() = "tests" + ) +} + +/** + * Returns the name of the write target field. + */ +string getSinkTargetFieldName(DataFlow::PathNode sink) { + result = any(DataFlow::Field fld | fld.getAWrite().getRhs() = sink.getNode()).getName() +} + +/** + * Returns the name of a ValueEntity. + */ +string getSourceValueEntityName(DataFlow::PathNode source) { + result = + any(DataFlow::ValueEntity val | source.getNode().(DataFlow::ReadNode).reads(val)).getName() +} + +/** + * Flow of unsecure TLS versions into a `tls.Config` struct, + * to the `MinVersion` and `MaxVersion` fields. + */ +class TlsVersionFlowConfig extends TaintTracking::Configuration { + TlsVersionFlowConfig() { this = "TlsVersionFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() = any(DataFlow::ValueExpr val | val.getIntValue() = [0]) or + source = + any(DataFlow::ValueEntity val | + val.hasQualifiedName("crypto/tls", ["VersionSSL30", "VersionTLS10", "VersionTLS11"]) + ).getARead() + } + + override predicate isSink(DataFlow::Node sink) { + exists(Write write | + write = + any(DataFlow::Field fld | + fld.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) + ).getAWrite() and + // The write must NOT happen inside a switch statement: + not exists(ExpressionSwitchStmt switch | + switch.getBody().getAChildStmt().getChild(0) = + any(Assignment asign | asign.getRhs() = sink.asExpr()) + ) + | + sink = write.getRhs() + ) + } + + override predicate isSanitizer(DataFlow::Node node) { isTestFile(node) } +} + +/** + * Find insecure TLS versions. + */ +predicate checkTlsVersions(DataFlow::PathNode source, DataFlow::PathNode sink, string message) { + exists(TlsVersionFlowConfig cfg | + cfg.hasFlowPath(source, sink) and + // Exclude tls.Config.Max = 0 (which is OK): + not exists(Write write, DataFlow::ValueExpr v0 | write.getRhs() = sink.getNode() | + v0 = source.getNode().asExpr() and + v0.getIntValue() = 0 and + exists(DataFlow::Field fld | + fld.hasQualifiedName("crypto/tls", "Config", "MaxVersion") and + fld.getAWrite().getRhs() = sink.getNode() + ) + ) + | + message = + "TLS version too low for " + getSinkTargetFieldName(sink) + ": " + + getSourceValueEntityName(source) + or + message = "Using lowest TLS version for " + getSinkTargetFieldName(sink) and + exists(DataFlow::ValueExpr v0 | + v0 = sink.getNode().asExpr() and + v0.getIntValue() = 0 + ) + ) +} + +/** + * Flow of unsecure TLS cipher suites into a `tls.Config` struct, + * to the `CipherSuites` field. + */ +class TlsInsecureCipherSuitesFlowConfig extends TaintTracking::Configuration { + TlsInsecureCipherSuitesFlowConfig() { this = "TlsInsecureCipherSuitesFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + // TODO: source can also be result of tls.InsecureCipherSuites()[0].ID + source = + any(DataFlow::FieldReadNode fieldRead | + fieldRead.getBase().getAPredecessor*() = + any(Function insecureCipherSuites | + insecureCipherSuites.hasQualifiedName("crypto/tls", "InsecureCipherSuites") + ).getACall().getResult() and + fieldRead.getFieldName() = "ID" + ) + or + source = + any(DataFlow::ValueEntity val | + val + .hasQualifiedName("crypto/tls", + any(string suiteName | + suiteName = "TLS_RSA_WITH_RC4_128_SHA" or + suiteName = "TLS_RSA_WITH_AES_128_CBC_SHA256" or + suiteName = "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA" or + suiteName = "TLS_ECDHE_RSA_WITH_RC4_128_SHA" or + suiteName = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" or + suiteName = "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256" + )) + ).getARead() + } + + override predicate isSink(DataFlow::Node sink) { + sink = + any(DataFlow::Field fld | fld.hasQualifiedName("crypto/tls", "Config", "CipherSuites")) + .getAWrite() + .getRhs() + } + + override predicate isSanitizer(DataFlow::Node node) { isTestFile(node) } +} + +/** + * Find insecure TLS cipher suites. + */ +predicate checkTlsInsecureCipherSuites( + DataFlow::PathNode source, DataFlow::PathNode sink, string message +) { + exists(TlsInsecureCipherSuitesFlowConfig cfg | cfg.hasFlowPath(source, sink) | + message = "Use of an insecure cipher suite: " + getSourceValueEntityName(source) + ) +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, string message +where + checkTlsVersions(source, sink, message) or + checkTlsInsecureCipherSuites(source, sink, message) +select sink.getNode(), source, sink, message diff --git a/ql/src/experimental/CWE-327/SaferTLS.go b/ql/src/experimental/CWE-327/SaferTLS.go new file mode 100644 index 00000000000..b3e9fce2a4f --- /dev/null +++ b/ql/src/experimental/CWE-327/SaferTLS.go @@ -0,0 +1,11 @@ +package main + +import "crypto/tls" + +func saferTLSConfig() { + config := &tls.Config{} + config.MinVersion = tls.VersionTLS12 + config.MaxVersion = tls.VersionTLS13 + // OR + config.MaxVersion = 0 // Setting MaxVersion to 0 means that the highest version available in the package will be used. +} diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.expected b/ql/test/experimental/CWE-327/UnsafeTLS.expected new file mode 100644 index 00000000000..c6591fdc957 --- /dev/null +++ b/ql/test/experimental/CWE-327/UnsafeTLS.expected @@ -0,0 +1,100 @@ +edges +| UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | +| UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:103:18:105:4 | slice literal | +| UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:111:18:113:4 | slice literal | +| UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:119:18:121:4 | slice literal | +| UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:127:18:129:4 | slice literal | +| UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:135:18:137:4 | slice literal | +| UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:143:18:145:4 | slice literal | +| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | +| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | +| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | +| UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:160:25:160:94 | call to append : slice type | UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | +| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | UnsafeTLS.go:160:25:160:94 | call to append | +| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | UnsafeTLS.go:160:25:160:94 | call to append : slice type | +| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append | +| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append : slice type | +nodes +| UnsafeTLS.go:14:23:14:23 | 0 | semmle.label | 0 | +| UnsafeTLS.go:18:23:18:23 | 0 | semmle.label | 0 | +| UnsafeTLS.go:23:16:23:16 | 0 | semmle.label | 0 | +| UnsafeTLS.go:29:16:29:16 | 0 | semmle.label | 0 | +| UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | +| UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | +| UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:90:18:97:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:103:18:105:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:111:18:113:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:119:18:121:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:127:18:129:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:135:18:137:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:143:18:145:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | semmle.label | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:25:160:94 | call to append | semmle.label | call to append | +| UnsafeTLS.go:160:25:160:94 | call to append : slice type | semmle.label | call to append : slice type | +| UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | semmle.label | selection of CipherSuites : slice type | +| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +#select +| UnsafeTLS.go:14:23:14:23 | 0 | UnsafeTLS.go:14:23:14:23 | 0 | UnsafeTLS.go:14:23:14:23 | 0 | Using lowest TLS version for MinVersion | +| UnsafeTLS.go:23:16:23:16 | 0 | UnsafeTLS.go:23:16:23:16 | 0 | UnsafeTLS.go:23:16:23:16 | 0 | Using lowest TLS version for MinVersion | +| UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | TLS version too low for MinVersion: VersionSSL30 | +| UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | TLS version too low for MaxVersion: VersionSSL30 | +| UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | +| UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | TLS version too low for MaxVersion: VersionTLS10 | +| UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | +| UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:103:18:105:4 | slice literal | UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:103:18:105:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:111:18:113:4 | slice literal | UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:111:18:113:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:119:18:121:4 | slice literal | UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:119:18:121:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:127:18:129:4 | slice literal | UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:127:18:129:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:135:18:137:4 | slice literal | UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:135:18:137:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:143:18:145:4 | slice literal | UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:143:18:145:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:160:25:160:94 | call to append | UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | +| UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go new file mode 100644 index 00000000000..6c7c206eb8d --- /dev/null +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -0,0 +1,214 @@ +package main + +import ( + "crypto/tls" +) + +func main() { + +} + +func minMaxTlsVersion() { + { + config := &tls.Config{} + config.MinVersion = 0 //BAD + } + { + config := &tls.Config{} + config.MaxVersion = 0 //GOOD + } + /// + { + config := &tls.Config{ + MinVersion: 0, //BAD + } + _ = config + } + { + config := &tls.Config{ + MaxVersion: 0, //GOOD + } + _ = config + } + /// + { + config := &tls.Config{} + config.MinVersion = tls.VersionSSL30 //BAD + } + { + config := &tls.Config{} + config.MaxVersion = tls.VersionSSL30 //BAD + } + /// + { + config := &tls.Config{} + config.MinVersion = tls.VersionTLS10 //BAD + } + { + config := &tls.Config{} + config.MaxVersion = tls.VersionTLS10 //BAD + } + /// + { + config := &tls.Config{} + config.MinVersion = tls.VersionTLS11 //BAD + } + { + config := &tls.Config{} + config.MaxVersion = tls.VersionTLS11 //BAD + } + /// + { + config := &tls.Config{ + MinVersion: tls.VersionTLS11, //BAD + } + _ = config + } + { + config := &tls.Config{ + MaxVersion: tls.VersionTLS11, //BAD + } + _ = config + } + { + config := &tls.Config{ + MinVersion: tls.VersionTLS12, //GOOD + } + _ = config + } + { + config := &tls.Config{ + MaxVersion: tls.VersionTLS13, //GOOD + } + _ = config + } +} + +func cipherSuites() { + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_RSA_WITH_RC4_128_SHA, //BAD + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, //BAD + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, //BAD + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_RSA_WITH_RC4_128_SHA, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, //BAD + }, + } + _ = config + } + { + config := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_CHACHA20_POLY1305_SHA256, //GOOD + }, + } + _ = config + } + { + config := &tls.Config{} + config.CipherSuites = make([]uint16, 0) + config.CipherSuites = append(config.CipherSuites, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) //BAD + } + { + config := &tls.Config{} + config.CipherSuites = make([]uint16, 0) + insecureSuites := tls.InsecureCipherSuites() + for _, v := range insecureSuites { + config.CipherSuites = append(config.CipherSuites, v.ID) //BAD + } + } +} + +func good(version string) { + config := &tls.Config{} + + switch version { + case "1.0": + config.MinVersion = tls.VersionTLS10 //OK + case "1.1": + config.MinVersion = tls.VersionTLS11 //OK + default: + config.MinVersion = tls.VersionTLS12 //OK + } + + _ = config +} +func badTlsVersion2(version string) { + { + config := &tls.Config{} + + switch version { + case "1.0": + config.MinVersion = tls.VersionTLS10 //OK + case "1.1": + config.MinVersion = tls.VersionTLS11 //OK + default: + config.MinVersion = tls.VersionTLS12 //OK + } + + config.MinVersion = tls.VersionTLS11 //BAD + + _ = config + } + { + + switch version { + case "1.0": + config := &tls.Config{} + config.MinVersion = tls.VersionTLS10 //BAD + config.MinVersion = tls.VersionTLS11 //BAD + _ = config + } + + } +} diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.qlref b/ql/test/experimental/CWE-327/UnsafeTLS.qlref new file mode 100644 index 00000000000..9b614dde9fa --- /dev/null +++ b/ql/test/experimental/CWE-327/UnsafeTLS.qlref @@ -0,0 +1 @@ +experimental/CWE-327/InsecureTLS.ql From e38d4ecd9c709d0f1138d5a91137cde5d6a94d4e Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 17:00:31 +0300 Subject: [PATCH 02/11] Fix typos --- ql/src/experimental/CWE-327/InsecureTLS.go | 2 +- ql/src/experimental/CWE-327/InsecureTLS.qhelp | 2 +- ql/src/experimental/CWE-327/SaferTLS.go | 2 +- ql/test/experimental/CWE-327/UnsafeTLS.go | 76 +++++++++---------- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.go b/ql/src/experimental/CWE-327/InsecureTLS.go index 4d78dbc2aa9..000eec9afb1 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.go +++ b/ql/src/experimental/CWE-327/InsecureTLS.go @@ -9,7 +9,7 @@ func main() {} func insecureMinMaxTlsVersion() { { config := &tls.Config{} - config.MinVersion = 0 //BAD: Setting the MinVersion to 0 equal to choosing the lowest supported version (i.e. SSL3.0) + config.MinVersion = 0 //BAD: Setting the MinVersion to 0 equals to choosing the lowest supported version (i.e. SSL3.0) } { config := &tls.Config{} diff --git a/ql/src/experimental/CWE-327/InsecureTLS.qhelp b/ql/src/experimental/CWE-327/InsecureTLS.qhelp index fcdb3d8875b..7df7c9f8a2a 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.qhelp +++ b/ql/src/experimental/CWE-327/InsecureTLS.qhelp @@ -44,4 +44,4 @@ Transport Layer Protection Cheat Sheet - \ No newline at end of file + diff --git a/ql/src/experimental/CWE-327/SaferTLS.go b/ql/src/experimental/CWE-327/SaferTLS.go index b3e9fce2a4f..d4cd7ecd17c 100644 --- a/ql/src/experimental/CWE-327/SaferTLS.go +++ b/ql/src/experimental/CWE-327/SaferTLS.go @@ -7,5 +7,5 @@ func saferTLSConfig() { config.MinVersion = tls.VersionTLS12 config.MaxVersion = tls.VersionTLS13 // OR - config.MaxVersion = 0 // Setting MaxVersion to 0 means that the highest version available in the package will be used. + config.MaxVersion = 0 // GOOD: Setting MaxVersion to 0 means that the highest version available in the package will be used. } diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index 6c7c206eb8d..46f844d4940 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -11,74 +11,74 @@ func main() { func minMaxTlsVersion() { { config := &tls.Config{} - config.MinVersion = 0 //BAD + config.MinVersion = 0 // BAD } { config := &tls.Config{} - config.MaxVersion = 0 //GOOD + config.MaxVersion = 0 // GOOD } /// { config := &tls.Config{ - MinVersion: 0, //BAD + MinVersion: 0, // BAD } _ = config } { config := &tls.Config{ - MaxVersion: 0, //GOOD + MaxVersion: 0, // GOOD } _ = config } /// { config := &tls.Config{} - config.MinVersion = tls.VersionSSL30 //BAD + config.MinVersion = tls.VersionSSL30 // BAD } { config := &tls.Config{} - config.MaxVersion = tls.VersionSSL30 //BAD + config.MaxVersion = tls.VersionSSL30 // BAD } /// { config := &tls.Config{} - config.MinVersion = tls.VersionTLS10 //BAD + config.MinVersion = tls.VersionTLS10 // BAD } { config := &tls.Config{} - config.MaxVersion = tls.VersionTLS10 //BAD + config.MaxVersion = tls.VersionTLS10 // BAD } /// { config := &tls.Config{} - config.MinVersion = tls.VersionTLS11 //BAD + config.MinVersion = tls.VersionTLS11 // BAD } { config := &tls.Config{} - config.MaxVersion = tls.VersionTLS11 //BAD + config.MaxVersion = tls.VersionTLS11 // BAD } /// { config := &tls.Config{ - MinVersion: tls.VersionTLS11, //BAD + MinVersion: tls.VersionTLS11, // BAD } _ = config } { config := &tls.Config{ - MaxVersion: tls.VersionTLS11, //BAD + MaxVersion: tls.VersionTLS11, // BAD } _ = config } { config := &tls.Config{ - MinVersion: tls.VersionTLS12, //GOOD + MinVersion: tls.VersionTLS12, // GOOD } _ = config } { config := &tls.Config{ - MaxVersion: tls.VersionTLS13, //GOOD + MaxVersion: tls.VersionTLS13, // GOOD } _ = config } @@ -88,12 +88,12 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_RSA_WITH_RC4_128_SHA, //BAD - tls.TLS_RSA_WITH_AES_128_CBC_SHA256, //BAD - tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, //BAD - tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, //BAD - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, //BAD - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_RSA_WITH_RC4_128_SHA, // BAD + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, // BAD + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, // BAD + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, // BAD + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, // BAD + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, // BAD }, } _ = config @@ -101,7 +101,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_RSA_WITH_RC4_128_SHA, //BAD + tls.TLS_RSA_WITH_RC4_128_SHA, // BAD }, } _ = config @@ -109,7 +109,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_RSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, // BAD }, } _ = config @@ -117,7 +117,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, //BAD + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, // BAD }, } _ = config @@ -125,7 +125,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, //BAD + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, // BAD }, } _ = config @@ -133,7 +133,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, // BAD }, } _ = config @@ -141,7 +141,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, //BAD + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, // BAD }, } _ = config @@ -149,7 +149,7 @@ func cipherSuites() { { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_CHACHA20_POLY1305_SHA256, //GOOD + tls.TLS_CHACHA20_POLY1305_SHA256, // GOOD }, } _ = config @@ -157,14 +157,14 @@ func cipherSuites() { { config := &tls.Config{} config.CipherSuites = make([]uint16, 0) - config.CipherSuites = append(config.CipherSuites, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) //BAD + config.CipherSuites = append(config.CipherSuites, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) // BAD } { config := &tls.Config{} config.CipherSuites = make([]uint16, 0) insecureSuites := tls.InsecureCipherSuites() for _, v := range insecureSuites { - config.CipherSuites = append(config.CipherSuites, v.ID) //BAD + config.CipherSuites = append(config.CipherSuites, v.ID) // BAD } } } @@ -174,11 +174,11 @@ func good(version string) { switch version { case "1.0": - config.MinVersion = tls.VersionTLS10 //OK + config.MinVersion = tls.VersionTLS10 // OK case "1.1": - config.MinVersion = tls.VersionTLS11 //OK + config.MinVersion = tls.VersionTLS11 // OK default: - config.MinVersion = tls.VersionTLS12 //OK + config.MinVersion = tls.VersionTLS12 // OK } _ = config @@ -189,14 +189,14 @@ func badTlsVersion2(version string) { switch version { case "1.0": - config.MinVersion = tls.VersionTLS10 //OK + config.MinVersion = tls.VersionTLS10 // OK case "1.1": - config.MinVersion = tls.VersionTLS11 //OK + config.MinVersion = tls.VersionTLS11 // OK default: - config.MinVersion = tls.VersionTLS12 //OK + config.MinVersion = tls.VersionTLS12 // OK } - config.MinVersion = tls.VersionTLS11 //BAD + config.MinVersion = tls.VersionTLS11 // BAD _ = config } @@ -205,8 +205,8 @@ func badTlsVersion2(version string) { switch version { case "1.0": config := &tls.Config{} - config.MinVersion = tls.VersionTLS10 //BAD - config.MinVersion = tls.VersionTLS11 //BAD + config.MinVersion = tls.VersionTLS10 // BAD + config.MinVersion = tls.VersionTLS11 // BAD _ = config } From 783f7101886b959c70076e6bfb73a834791ba3b0 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 17:12:15 +0300 Subject: [PATCH 03/11] Fix comments --- ql/src/experimental/CWE-327/InsecureTLS.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.go b/ql/src/experimental/CWE-327/InsecureTLS.go index 000eec9afb1..5163fd641d9 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.go +++ b/ql/src/experimental/CWE-327/InsecureTLS.go @@ -9,22 +9,22 @@ func main() {} func insecureMinMaxTlsVersion() { { config := &tls.Config{} - config.MinVersion = 0 //BAD: Setting the MinVersion to 0 equals to choosing the lowest supported version (i.e. SSL3.0) + config.MinVersion = 0 // BAD: Setting the MinVersion to 0 equals to choosing the lowest supported version (i.e. SSL3.0) } { config := &tls.Config{} - config.MinVersion = tls.VersionSSL30 //BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MinVersion. + config.MinVersion = tls.VersionSSL30 // BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MinVersion. } { config := &tls.Config{} - config.MaxVersion = tls.VersionSSL30 //BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MaxVersion. + config.MaxVersion = tls.VersionSSL30 // BAD: SSL 3.0 is a non-secure version of the protocol; it's not safe to use it as MaxVersion. } } func insecureCipherSuites() { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_RSA_WITH_RC4_128_SHA, //BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure chiper suites; it's not safe to be used. + tls.TLS_RSA_WITH_RC4_128_SHA, // BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure chiper suites; it's not safe to be used. }, } _ = config From 70bc4c81a08984007c94e9f524ea751f3ed82f4e Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 17:15:56 +0300 Subject: [PATCH 04/11] Fix typo --- ql/src/experimental/CWE-327/InsecureTLS.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.go b/ql/src/experimental/CWE-327/InsecureTLS.go index 5163fd641d9..41235c8b4ee 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.go +++ b/ql/src/experimental/CWE-327/InsecureTLS.go @@ -24,7 +24,7 @@ func insecureMinMaxTlsVersion() { func insecureCipherSuites() { config := &tls.Config{ CipherSuites: []uint16{ - tls.TLS_RSA_WITH_RC4_128_SHA, // BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure chiper suites; it's not safe to be used. + tls.TLS_RSA_WITH_RC4_128_SHA, // BAD: TLS_RSA_WITH_RC4_128_SHA is one of the non-secure cipher suites; it's not safe to be used. }, } _ = config From 29eba441d70cfca3dd1365e38ce1abc59220daed Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 17:50:20 +0300 Subject: [PATCH 05/11] Determine TLS version from int value --- ql/src/experimental/CWE-327/InsecureTLS.ql | 36 +++- .../experimental/CWE-327/UnsafeTLS.expected | 198 +++++++++--------- ql/test/experimental/CWE-327/UnsafeTLS.go | 17 +- 3 files changed, 145 insertions(+), 106 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql index 0f7af28e035..bc419b93bab 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.ql +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -37,6 +37,28 @@ string getSourceValueEntityName(DataFlow::PathNode source) { any(DataFlow::ValueEntity val | source.getNode().(DataFlow::ReadNode).reads(val)).getName() } +predicate isUnsafeTlsVersionInt(int val) { + // tls.VersionSSL30 + val = 768 + or + // tls.VersionTLS10 + val = 769 + or + // tls.VersionTLS11 + val = 770 +} + +string tlsVersionIntToString(int val) { + // tls.VersionSSL30 + val = 768 and result = "VersionSSL30" + or + // tls.VersionTLS10 + val = 769 and result = "VersionTLS10" + or + // tls.VersionTLS11 + val = 770 and result = "VersionTLS11" +} + /** * Flow of unsecure TLS versions into a `tls.Config` struct, * to the `MinVersion` and `MaxVersion` fields. @@ -45,11 +67,10 @@ class TlsVersionFlowConfig extends TaintTracking::Configuration { TlsVersionFlowConfig() { this = "TlsVersionFlowConfig" } override predicate isSource(DataFlow::Node source) { - source.asExpr() = any(DataFlow::ValueExpr val | val.getIntValue() = [0]) or - source = - any(DataFlow::ValueEntity val | - val.hasQualifiedName("crypto/tls", ["VersionSSL30", "VersionTLS10", "VersionTLS11"]) - ).getARead() + source.asExpr() = + any(DataFlow::ValueExpr val | + val.getIntValue() = 0 or isUnsafeTlsVersionInt(val.getIntValue()) + ) } override predicate isSink(DataFlow::Node sink) { @@ -89,7 +110,10 @@ predicate checkTlsVersions(DataFlow::PathNode source, DataFlow::PathNode sink, s | message = "TLS version too low for " + getSinkTargetFieldName(sink) + ": " + - getSourceValueEntityName(source) + tlsVersionIntToString(any(DataFlow::ValueExpr val | + val = sink.getNode().asExpr() and + val.getIntValue() != 0 + ).getIntValue()) or message = "Using lowest TLS version for " + getSinkTargetFieldName(sink) and exists(DataFlow::ValueExpr v0 | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.expected b/ql/test/experimental/CWE-327/UnsafeTLS.expected index c6591fdc957..4bf03f3a034 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.expected +++ b/ql/test/experimental/CWE-327/UnsafeTLS.expected @@ -1,100 +1,104 @@ edges -| UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | -| UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:103:18:105:4 | slice literal | -| UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:111:18:113:4 | slice literal | -| UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:119:18:121:4 | slice literal | -| UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:127:18:129:4 | slice literal | -| UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:135:18:137:4 | slice literal | -| UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:143:18:145:4 | slice literal | -| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | -| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | -| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | -| UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | -| UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | -| UnsafeTLS.go:160:25:160:94 | call to append : slice type | UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | -| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | UnsafeTLS.go:160:25:160:94 | call to append | -| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | UnsafeTLS.go:160:25:160:94 | call to append : slice type | -| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append | -| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append : slice type | +| UnsafeTLS.go:102:5:102:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:103:5:103:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:104:5:104:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:105:5:105:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:106:5:106:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:107:5:107:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | +| UnsafeTLS.go:115:5:115:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:114:18:116:4 | slice literal | +| UnsafeTLS.go:123:5:123:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:122:18:124:4 | slice literal | +| UnsafeTLS.go:131:5:131:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:130:18:132:4 | slice literal | +| UnsafeTLS.go:139:5:139:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:138:18:140:4 | slice literal | +| UnsafeTLS.go:147:5:147:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:146:18:148:4 | slice literal | +| UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:154:18:156:4 | slice literal | +| UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:170:3:170:8 | config [pointer, CipherSuites] | +| UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:171:3:171:8 | config [pointer, CipherSuites] | +| UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | UnsafeTLS.go:171:32:171:37 | config [pointer, CipherSuites] | +| UnsafeTLS.go:170:3:170:8 | config [pointer, CipherSuites] | UnsafeTLS.go:170:3:170:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:170:3:170:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:171:3:171:8 | config [pointer, CipherSuites] | UnsafeTLS.go:171:3:171:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:3:171:8 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:171:25:171:94 | call to append : slice type | UnsafeTLS.go:171:3:171:8 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:32:171:37 | config [pointer, CipherSuites] | UnsafeTLS.go:171:32:171:37 | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:32:171:37 | implicit dereference [CipherSuites] : slice type | UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | +| UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | UnsafeTLS.go:171:25:171:94 | call to append | +| UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | UnsafeTLS.go:171:25:171:94 | call to append : slice type | +| UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | +| UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append : slice type | nodes -| UnsafeTLS.go:14:23:14:23 | 0 | semmle.label | 0 | -| UnsafeTLS.go:18:23:18:23 | 0 | semmle.label | 0 | -| UnsafeTLS.go:23:16:23:16 | 0 | semmle.label | 0 | -| UnsafeTLS.go:29:16:29:16 | 0 | semmle.label | 0 | -| UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | -| UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | -| UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | -| UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | -| UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:90:18:97:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:103:18:105:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:111:18:113:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:119:18:121:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:127:18:129:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | -| UnsafeTLS.go:135:18:137:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:143:18:145:4 | slice literal | semmle.label | slice literal | -| UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:158:3:158:8 | definition of config [pointer, CipherSuites] | semmle.label | definition of config [pointer, CipherSuites] | -| UnsafeTLS.go:159:3:159:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | -| UnsafeTLS.go:159:3:159:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:3:160:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | -| UnsafeTLS.go:160:3:160:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:25:160:94 | call to append | semmle.label | call to append | -| UnsafeTLS.go:160:25:160:94 | call to append : slice type | semmle.label | call to append : slice type | -| UnsafeTLS.go:160:32:160:37 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | -| UnsafeTLS.go:160:32:160:37 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | -| UnsafeTLS.go:160:32:160:50 | selection of CipherSuites : slice type | semmle.label | selection of CipherSuites : slice type | -| UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | -| UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:12:23:12:23 | 0 | semmle.label | 0 | +| UnsafeTLS.go:16:23:16:23 | 0 | semmle.label | 0 | +| UnsafeTLS.go:21:16:21:16 | 0 | semmle.label | 0 | +| UnsafeTLS.go:27:16:27:16 | 0 | semmle.label | 0 | +| UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | +| UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | semmle.label | selection of VersionSSL30 | +| UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:86:16:86:21 | 0x0300 | semmle.label | 0x0300 | +| UnsafeTLS.go:92:16:92:21 | 0x0301 | semmle.label | 0x0301 | +| UnsafeTLS.go:101:18:108:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:102:5:102:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:103:5:103:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:104:5:104:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:105:5:105:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:106:5:106:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:107:5:107:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:114:18:116:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:115:5:115:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:122:18:124:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:123:5:123:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:130:18:132:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:131:5:131:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:138:18:140:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:139:5:139:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | +| UnsafeTLS.go:146:18:148:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:147:5:147:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:154:18:156:4 | slice literal | semmle.label | slice literal | +| UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:169:3:169:8 | definition of config [pointer, CipherSuites] | semmle.label | definition of config [pointer, CipherSuites] | +| UnsafeTLS.go:170:3:170:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:170:3:170:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:3:171:8 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:171:3:171:8 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:25:171:94 | call to append | semmle.label | call to append | +| UnsafeTLS.go:171:25:171:94 | call to append : slice type | semmle.label | call to append : slice type | +| UnsafeTLS.go:171:32:171:37 | config [pointer, CipherSuites] | semmle.label | config [pointer, CipherSuites] | +| UnsafeTLS.go:171:32:171:37 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | +| UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | semmle.label | selection of CipherSuites : slice type | +| UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | +| UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | #select -| UnsafeTLS.go:14:23:14:23 | 0 | UnsafeTLS.go:14:23:14:23 | 0 | UnsafeTLS.go:14:23:14:23 | 0 | Using lowest TLS version for MinVersion | -| UnsafeTLS.go:23:16:23:16 | 0 | UnsafeTLS.go:23:16:23:16 | 0 | UnsafeTLS.go:23:16:23:16 | 0 | Using lowest TLS version for MinVersion | -| UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | UnsafeTLS.go:36:23:36:38 | selection of VersionSSL30 | TLS version too low for MinVersion: VersionSSL30 | -| UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | UnsafeTLS.go:40:23:40:38 | selection of VersionSSL30 | TLS version too low for MaxVersion: VersionSSL30 | -| UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | UnsafeTLS.go:45:23:45:38 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | -| UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | UnsafeTLS.go:49:23:49:38 | selection of VersionTLS10 | TLS version too low for MaxVersion: VersionTLS10 | -| UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | UnsafeTLS.go:54:23:54:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | UnsafeTLS.go:58:23:58:38 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | -| UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | UnsafeTLS.go:63:16:63:31 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | UnsafeTLS.go:69:16:69:31 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:91:5:91:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:92:5:92:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:93:5:93:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:94:5:94:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:95:5:95:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:90:18:97:4 | slice literal | UnsafeTLS.go:96:5:96:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:90:18:97:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:103:18:105:4 | slice literal | UnsafeTLS.go:104:5:104:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:103:18:105:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:111:18:113:4 | slice literal | UnsafeTLS.go:112:5:112:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:111:18:113:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:119:18:121:4 | slice literal | UnsafeTLS.go:120:5:120:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:119:18:121:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:127:18:129:4 | slice literal | UnsafeTLS.go:128:5:128:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:127:18:129:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:135:18:137:4 | slice literal | UnsafeTLS.go:136:5:136:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:135:18:137:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:143:18:145:4 | slice literal | UnsafeTLS.go:144:5:144:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:143:18:145:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:160:25:160:94 | call to append | UnsafeTLS.go:160:53:160:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:160:25:160:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | UnsafeTLS.go:199:23:199:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | UnsafeTLS.go:208:24:208:39 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | -| UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | UnsafeTLS.go:209:24:209:39 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | Using lowest TLS version for MinVersion | +| UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | Using lowest TLS version for MinVersion | +| UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | TLS version too low for MinVersion: VersionSSL30 | +| UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | TLS version too low for MaxVersion: VersionSSL30 | +| UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | +| UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | TLS version too low for MaxVersion: VersionTLS10 | +| UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | +| UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | +| UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | TLS version too low for MinVersion: VersionSSL30 | +| UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | TLS version too low for MaxVersion: VersionTLS10 | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:102:5:102:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:103:5:103:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:104:5:104:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:105:5:105:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:106:5:106:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:107:5:107:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:114:18:116:4 | slice literal | UnsafeTLS.go:115:5:115:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:114:18:116:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:122:18:124:4 | slice literal | UnsafeTLS.go:123:5:123:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:122:18:124:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:130:18:132:4 | slice literal | UnsafeTLS.go:131:5:131:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:130:18:132:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:138:18:140:4 | slice literal | UnsafeTLS.go:139:5:139:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:138:18:140:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | +| UnsafeTLS.go:146:18:148:4 | slice literal | UnsafeTLS.go:147:5:147:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:146:18:148:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:154:18:156:4 | slice literal | UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:154:18:156:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:171:25:171:94 | call to append | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | +| UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | +| UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index 46f844d4940..bb55a2e13ee 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -4,9 +4,7 @@ import ( "crypto/tls" ) -func main() { - -} +func main() {} func minMaxTlsVersion() { { @@ -82,6 +80,19 @@ func minMaxTlsVersion() { } _ = config } + /// + { + config := &tls.Config{ + MinVersion: 0x0300, // BAD + } + _ = config + } + { + config := &tls.Config{ + MaxVersion: 0x0301, // BAD + } + _ = config + } } func cipherSuites() { From 4ab929a65688b608c235a7bf648975778ca48b3f Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 22 Jun 2020 17:54:07 +0300 Subject: [PATCH 06/11] Simplify --- ql/src/experimental/CWE-327/InsecureTLS.ql | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql index bc419b93bab..1c7d21098e3 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.ql +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -102,10 +102,7 @@ predicate checkTlsVersions(DataFlow::PathNode source, DataFlow::PathNode sink, s not exists(Write write, DataFlow::ValueExpr v0 | write.getRhs() = sink.getNode() | v0 = source.getNode().asExpr() and v0.getIntValue() = 0 and - exists(DataFlow::Field fld | - fld.hasQualifiedName("crypto/tls", "Config", "MaxVersion") and - fld.getAWrite().getRhs() = sink.getNode() - ) + getSinkTargetFieldName(sink) = "MaxVersion" ) | message = From 56727b220bc51fb5afeff0a933bbf974a43cb519 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Tue, 23 Jun 2020 12:14:49 +0300 Subject: [PATCH 07/11] Try different ways of passing taint through a field --- ql/src/experimental/CWE-327/InsecureTLS.ql | 10 +++------- ql/test/experimental/CWE-327/UnsafeTLS.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql index 1c7d21098e3..dd5dd09284c 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.ql +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -130,13 +130,9 @@ class TlsInsecureCipherSuitesFlowConfig extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { // TODO: source can also be result of tls.InsecureCipherSuites()[0].ID source = - any(DataFlow::FieldReadNode fieldRead | - fieldRead.getBase().getAPredecessor*() = - any(Function insecureCipherSuites | - insecureCipherSuites.hasQualifiedName("crypto/tls", "InsecureCipherSuites") - ).getACall().getResult() and - fieldRead.getFieldName() = "ID" - ) + any(Function insecureCipherSuites | + insecureCipherSuites.hasQualifiedName("crypto/tls", "InsecureCipherSuites") + ).getACall().getResult() or source = any(DataFlow::ValueEntity val | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index bb55a2e13ee..1ac8dc39456 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -178,6 +178,24 @@ func cipherSuites() { config.CipherSuites = append(config.CipherSuites, v.ID) // BAD } } + { + config := &tls.Config{} + cipherSuites := make([]uint16, 0) + insecureSuites := tls.InsecureCipherSuites() + for _, v := range insecureSuites { + cipherSuites = append(cipherSuites, v.ID) + } + config.CipherSuites = cipherSuites // BAD + } + { + config := &tls.Config{} + cipherSuites := make([]uint16, 0) + insecureSuites := tls.InsecureCipherSuites() + for i := range insecureSuites { + cipherSuites = append(cipherSuites, insecureSuites[i].ID) + } + config.CipherSuites = cipherSuites // BAD + } } func good(version string) { From 561c5b91d2029a57d50528fe1d53c9da69f460f4 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Tue, 23 Jun 2020 16:07:05 +0300 Subject: [PATCH 08/11] Implement code review feedback --- ql/src/experimental/CWE-327/InsecureTLS.ql | 168 ++++++++---------- .../experimental/CWE-327/UnsafeTLS.expected | 72 ++++---- ql/test/experimental/CWE-327/UnsafeTLS.go | 1 + 3 files changed, 120 insertions(+), 121 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql index dd5dd09284c..ef193d161c9 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.ql +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -22,41 +22,19 @@ predicate isTestFile(DataFlow::Node node) { ) } -/** - * Returns the name of the write target field. - */ -string getSinkTargetFieldName(DataFlow::PathNode sink) { - result = any(DataFlow::Field fld | fld.getAWrite().getRhs() = sink.getNode()).getName() -} - -/** - * Returns the name of a ValueEntity. - */ -string getSourceValueEntityName(DataFlow::PathNode source) { - result = - any(DataFlow::ValueEntity val | source.getNode().(DataFlow::ReadNode).reads(val)).getName() -} - -predicate isUnsafeTlsVersionInt(int val) { +predicate unsafeTlsVersion(int val, string name) { // tls.VersionSSL30 - val = 768 + val = 768 and name = "VersionSSL30" or // tls.VersionTLS10 - val = 769 + val = 769 and name = "VersionTLS10" or // tls.VersionTLS11 - val = 770 -} - -string tlsVersionIntToString(int val) { - // tls.VersionSSL30 - val = 768 and result = "VersionSSL30" + val = 770 and name = "VersionTLS11" or - // tls.VersionTLS10 - val = 769 and result = "VersionTLS10" - or - // tls.VersionTLS11 - val = 770 and result = "VersionTLS11" + // Zero indicates the lowest available version setting for MinVersion, + // or the highest available version setting for MaxVersion. + val = 0 and name = "" } /** @@ -66,56 +44,46 @@ string tlsVersionIntToString(int val) { class TlsVersionFlowConfig extends TaintTracking::Configuration { TlsVersionFlowConfig() { this = "TlsVersionFlowConfig" } - override predicate isSource(DataFlow::Node source) { - source.asExpr() = - any(DataFlow::ValueExpr val | - val.getIntValue() = 0 or isUnsafeTlsVersionInt(val.getIntValue()) - ) + predicate isSource(DataFlow::Node source, int val) { + val = source.getIntValue() and + unsafeTlsVersion(val, _) } - override predicate isSink(DataFlow::Node sink) { - exists(Write write | - write = - any(DataFlow::Field fld | - fld.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) - ).getAWrite() and - // The write must NOT happen inside a switch statement: - not exists(ExpressionSwitchStmt switch | - switch.getBody().getAChildStmt().getChild(0) = - any(Assignment asign | asign.getRhs() = sink.asExpr()) - ) - | - sink = write.getRhs() + predicate isSink(DataFlow::Node sink, Field f) { + f.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) and + sink = f.getAWrite().getRhs() and + // Exclude writes happening inside a switch statement, + // provided the config struct is not declared inside that same statement: + not exists(ExpressionSwitchStmt switch | + switch.getBody().getAChildStmt().getChild(0) = + any(Assignment asign | asign.getRhs() = sink.asExpr()) + // TODO: make sure that the parent struct of the field is not declared + // inside the switch statement. ) } - override predicate isSanitizer(DataFlow::Node node) { isTestFile(node) } + override predicate isSource(DataFlow::Node source) { isSource(source, _) } + + override predicate isSink(DataFlow::Node sink) { isSink(sink, _) } } /** * Find insecure TLS versions. */ predicate checkTlsVersions(DataFlow::PathNode source, DataFlow::PathNode sink, string message) { - exists(TlsVersionFlowConfig cfg | + exists(TlsVersionFlowConfig cfg, int version, Field fld | cfg.hasFlowPath(source, sink) and + cfg.isSource(source.getNode(), version) and + cfg.isSink(sink.getNode(), fld) and // Exclude tls.Config.Max = 0 (which is OK): - not exists(Write write, DataFlow::ValueExpr v0 | write.getRhs() = sink.getNode() | - v0 = source.getNode().asExpr() and - v0.getIntValue() = 0 and - getSinkTargetFieldName(sink) = "MaxVersion" - ) + not (version = 0 and fld.getName() = "MaxVersion") | - message = - "TLS version too low for " + getSinkTargetFieldName(sink) + ": " + - tlsVersionIntToString(any(DataFlow::ValueExpr val | - val = sink.getNode().asExpr() and - val.getIntValue() != 0 - ).getIntValue()) + version = 0 and + message = "Using lowest TLS version for " + fld + "." or - message = "Using lowest TLS version for " + getSinkTargetFieldName(sink) and - exists(DataFlow::ValueExpr v0 | - v0 = sink.getNode().asExpr() and - v0.getIntValue() = 0 + version != 0 and + exists(string name | unsafeTlsVersion(version, name) | + message = "Using insecure TLS version " + name + " for " + fld + "." ) ) } @@ -127,36 +95,47 @@ predicate checkTlsVersions(DataFlow::PathNode source, DataFlow::PathNode sink, s class TlsInsecureCipherSuitesFlowConfig extends TaintTracking::Configuration { TlsInsecureCipherSuitesFlowConfig() { this = "TlsInsecureCipherSuitesFlowConfig" } + predicate isSourceValueEntity(DataFlow::Node source, string suiteName) { + exists(DataFlow::ValueEntity val | + val.hasQualifiedName("crypto/tls", suiteName) and + ( + suiteName = "TLS_RSA_WITH_RC4_128_SHA" + or + suiteName = "TLS_RSA_WITH_AES_128_CBC_SHA256" + or + suiteName = "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA" + or + suiteName = "TLS_ECDHE_RSA_WITH_RC4_128_SHA" + or + suiteName = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" + or + suiteName = "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256" + ) + | + source = val.getARead() + ) + } + + predicate isSourceInsecureCipherSuites(DataFlow::Node source) { + exists(Function insecureCipherSuites | + insecureCipherSuites.hasQualifiedName("crypto/tls", "InsecureCipherSuites") + | + source = insecureCipherSuites.getACall().getResult() + ) + } + override predicate isSource(DataFlow::Node source) { // TODO: source can also be result of tls.InsecureCipherSuites()[0].ID - source = - any(Function insecureCipherSuites | - insecureCipherSuites.hasQualifiedName("crypto/tls", "InsecureCipherSuites") - ).getACall().getResult() + isSourceInsecureCipherSuites(source) or - source = - any(DataFlow::ValueEntity val | - val - .hasQualifiedName("crypto/tls", - any(string suiteName | - suiteName = "TLS_RSA_WITH_RC4_128_SHA" or - suiteName = "TLS_RSA_WITH_AES_128_CBC_SHA256" or - suiteName = "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA" or - suiteName = "TLS_ECDHE_RSA_WITH_RC4_128_SHA" or - suiteName = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" or - suiteName = "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256" - )) - ).getARead() + isSourceValueEntity(source, _) } override predicate isSink(DataFlow::Node sink) { - sink = - any(DataFlow::Field fld | fld.hasQualifiedName("crypto/tls", "Config", "CipherSuites")) - .getAWrite() - .getRhs() + exists(DataFlow::Field fld | fld.hasQualifiedName("crypto/tls", "Config", "CipherSuites") | + sink = fld.getAWrite().getRhs() + ) } - - override predicate isSanitizer(DataFlow::Node node) { isTestFile(node) } } /** @@ -166,12 +145,21 @@ predicate checkTlsInsecureCipherSuites( DataFlow::PathNode source, DataFlow::PathNode sink, string message ) { exists(TlsInsecureCipherSuitesFlowConfig cfg | cfg.hasFlowPath(source, sink) | - message = "Use of an insecure cipher suite: " + getSourceValueEntityName(source) + exists(string name | cfg.isSourceValueEntity(source.getNode(), name) | + message = "Use of an insecure cipher suite: " + name + "." + ) + or + cfg.isSourceInsecureCipherSuites(source.getNode()) and + message = "Use of an insecure cipher suite from InsecureCipherSuites()." ) } from DataFlow::PathNode source, DataFlow::PathNode sink, string message where - checkTlsVersions(source, sink, message) or - checkTlsInsecureCipherSuites(source, sink, message) + ( + checkTlsVersions(source, sink, message) or + checkTlsInsecureCipherSuites(source, sink, message) + ) and + // Exclude results in test code: + not isTestFile(sink.getNode()) select sink.getNode(), source, sink, message diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.expected b/ql/test/experimental/CWE-327/UnsafeTLS.expected index 4bf03f3a034..884229e3e06 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.expected +++ b/ql/test/experimental/CWE-327/UnsafeTLS.expected @@ -25,6 +25,10 @@ edges | UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | UnsafeTLS.go:171:25:171:94 | call to append : slice type | | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append : slice type | +| UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | +| UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | UnsafeTLS.go:197:25:197:36 | cipherSuites | +| UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | +| UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | UnsafeTLS.go:197:25:197:36 | cipherSuites | nodes | UnsafeTLS.go:12:23:12:23 | 0 | semmle.label | 0 | | UnsafeTLS.go:16:23:16:23 | 0 | semmle.label | 0 | @@ -70,35 +74,41 @@ nodes | UnsafeTLS.go:171:32:171:37 | implicit dereference [CipherSuites] : slice type | semmle.label | implicit dereference [CipherSuites] : slice type | | UnsafeTLS.go:171:32:171:50 | selection of CipherSuites : slice type | semmle.label | selection of CipherSuites : slice type | | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | semmle.label | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | -| UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | -| UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | semmle.label | call to InsecureCipherSuites : slice type | +| UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | semmle.label | implicit dereference : CipherSuite | +| UnsafeTLS.go:197:25:197:36 | cipherSuites | semmle.label | cipherSuites | +| UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | +| UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | +| UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | #select -| UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | Using lowest TLS version for MinVersion | -| UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | Using lowest TLS version for MinVersion | -| UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | TLS version too low for MinVersion: VersionSSL30 | -| UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | TLS version too low for MaxVersion: VersionSSL30 | -| UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | -| UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | TLS version too low for MaxVersion: VersionTLS10 | -| UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | -| UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | TLS version too low for MaxVersion: VersionTLS11 | -| UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | TLS version too low for MinVersion: VersionSSL30 | -| UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | TLS version too low for MaxVersion: VersionTLS10 | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:102:5:102:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:103:5:103:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:104:5:104:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:105:5:105:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:106:5:106:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:107:5:107:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:114:18:116:4 | slice literal | UnsafeTLS.go:115:5:115:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:114:18:116:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:122:18:124:4 | slice literal | UnsafeTLS.go:123:5:123:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:122:18:124:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:130:18:132:4 | slice literal | UnsafeTLS.go:131:5:131:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:130:18:132:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:138:18:140:4 | slice literal | UnsafeTLS.go:139:5:139:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:138:18:140:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA | -| UnsafeTLS.go:146:18:148:4 | slice literal | UnsafeTLS.go:147:5:147:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:146:18:148:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:154:18:156:4 | slice literal | UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:154:18:156:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:171:25:171:94 | call to append | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 | -| UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | UnsafeTLS.go:210:23:210:38 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | -| UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | UnsafeTLS.go:219:24:219:39 | selection of VersionTLS10 | TLS version too low for MinVersion: VersionTLS10 | -| UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | UnsafeTLS.go:220:24:220:39 | selection of VersionTLS11 | TLS version too low for MinVersion: VersionTLS11 | +| UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | Using lowest TLS version for MinVersion. | +| UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | Using lowest TLS version for MinVersion. | +| UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | UnsafeTLS.go:34:23:34:38 | selection of VersionSSL30 | Using insecure TLS version VersionSSL30 for MinVersion. | +| UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | UnsafeTLS.go:38:23:38:38 | selection of VersionSSL30 | Using insecure TLS version VersionSSL30 for MaxVersion. | +| UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | UnsafeTLS.go:43:23:43:38 | selection of VersionTLS10 | Using insecure TLS version VersionTLS10 for MinVersion. | +| UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | UnsafeTLS.go:47:23:47:38 | selection of VersionTLS10 | Using insecure TLS version VersionTLS10 for MaxVersion. | +| UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | UnsafeTLS.go:52:23:52:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | +| UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | UnsafeTLS.go:56:23:56:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MaxVersion. | +| UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | UnsafeTLS.go:61:16:61:31 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | +| UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | UnsafeTLS.go:67:16:67:31 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MaxVersion. | +| UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | UnsafeTLS.go:86:16:86:21 | 0x0300 | Using insecure TLS version VersionSSL30 for MinVersion. | +| UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | UnsafeTLS.go:92:16:92:21 | 0x0301 | Using insecure TLS version VersionTLS10 for MaxVersion. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:102:5:102:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:103:5:103:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:104:5:104:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:105:5:105:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:106:5:106:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:101:18:108:4 | slice literal | UnsafeTLS.go:107:5:107:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:101:18:108:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:114:18:116:4 | slice literal | UnsafeTLS.go:115:5:115:32 | selection of TLS_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:114:18:116:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:122:18:124:4 | slice literal | UnsafeTLS.go:123:5:123:39 | selection of TLS_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:122:18:124:4 | slice literal | Use of an insecure cipher suite: TLS_RSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:130:18:132:4 | slice literal | UnsafeTLS.go:131:5:131:40 | selection of TLS_ECDHE_ECDSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:130:18:132:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:138:18:140:4 | slice literal | UnsafeTLS.go:139:5:139:38 | selection of TLS_ECDHE_RSA_WITH_RC4_128_SHA : uint16 | UnsafeTLS.go:138:18:140:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_RC4_128_SHA. | +| UnsafeTLS.go:146:18:148:4 | slice literal | UnsafeTLS.go:147:5:147:47 | selection of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:146:18:148:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:154:18:156:4 | slice literal | UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:154:18:156:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:171:25:171:94 | call to append | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256. | +| UnsafeTLS.go:197:25:197:36 | cipherSuites | UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | UnsafeTLS.go:197:25:197:36 | cipherSuites | Use of an insecure cipher suite from InsecureCipherSuites(). | +| UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MaxVersion. | +| UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | +| UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | Using insecure TLS version VersionTLS10 for MinVersion. | +| UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index 1ac8dc39456..2105916d63d 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -204,6 +204,7 @@ func good(version string) { switch version { case "1.0": config.MinVersion = tls.VersionTLS10 // OK + config.MaxVersion = tls.VersionTLS11 // BAD case "1.1": config.MinVersion = tls.VersionTLS11 // OK default: From 3aa9b256734cb4be5edb81df82cde2f9ac4d5b01 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Tue, 23 Jun 2020 22:40:25 +0300 Subject: [PATCH 09/11] Fix comment --- ql/test/experimental/CWE-327/UnsafeTLS.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index 2105916d63d..7c29dbb3c16 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -204,7 +204,7 @@ func good(version string) { switch version { case "1.0": config.MinVersion = tls.VersionTLS10 // OK - config.MaxVersion = tls.VersionTLS11 // BAD + config.MaxVersion = tls.VersionTLS11 // OK case "1.1": config.MinVersion = tls.VersionTLS11 // OK default: From 4dc1399385f50b918e576b55f4d65495fad82574 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 24 Jun 2020 15:11:33 +0300 Subject: [PATCH 10/11] Update comments on the lines that have incorrect flagging --- ql/test/experimental/CWE-327/UnsafeTLS.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index 7c29dbb3c16..e08844e003e 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -175,7 +175,7 @@ func cipherSuites() { config.CipherSuites = make([]uint16, 0) insecureSuites := tls.InsecureCipherSuites() for _, v := range insecureSuites { - config.CipherSuites = append(config.CipherSuites, v.ID) // BAD + config.CipherSuites = append(config.CipherSuites, v.ID) // TODO: should be flagged as BAD. } } { @@ -185,7 +185,7 @@ func cipherSuites() { for _, v := range insecureSuites { cipherSuites = append(cipherSuites, v.ID) } - config.CipherSuites = cipherSuites // BAD + config.CipherSuites = cipherSuites // TODO: should be flagged as BAD. } { config := &tls.Config{} @@ -204,7 +204,7 @@ func good(version string) { switch version { case "1.0": config.MinVersion = tls.VersionTLS10 // OK - config.MaxVersion = tls.VersionTLS11 // OK + config.MaxVersion = tls.VersionTLS11 // TODO: should be OK, but it's flagged as bad. case "1.1": config.MinVersion = tls.VersionTLS11 // OK default: From 95b76dceca014544aaab200d571587701d28b9b9 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 24 Jun 2020 21:39:23 +0300 Subject: [PATCH 11/11] Remove check --- ql/src/experimental/CWE-327/InsecureTLS.ql | 18 +++----- .../experimental/CWE-327/UnsafeTLS.expected | 8 ---- ql/test/experimental/CWE-327/UnsafeTLS.go | 45 ------------------- 3 files changed, 5 insertions(+), 66 deletions(-) diff --git a/ql/src/experimental/CWE-327/InsecureTLS.ql b/ql/src/experimental/CWE-327/InsecureTLS.ql index ef193d161c9..e56c1495106 100644 --- a/ql/src/experimental/CWE-327/InsecureTLS.ql +++ b/ql/src/experimental/CWE-327/InsecureTLS.ql @@ -17,8 +17,8 @@ import DataFlow::PathGraph */ predicate isTestFile(DataFlow::Node node) { // Exclude results in test files: - exists(File fl | fl = node.getRoot().getFile() | - fl instanceof TestFile or fl.getPackageName() = "tests" + exists(File file | file = node.getRoot().getFile() | + file instanceof TestFile or file.getPackageName() = "tests" ) } @@ -49,17 +49,9 @@ class TlsVersionFlowConfig extends TaintTracking::Configuration { unsafeTlsVersion(val, _) } - predicate isSink(DataFlow::Node sink, Field f) { - f.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) and - sink = f.getAWrite().getRhs() and - // Exclude writes happening inside a switch statement, - // provided the config struct is not declared inside that same statement: - not exists(ExpressionSwitchStmt switch | - switch.getBody().getAChildStmt().getChild(0) = - any(Assignment asign | asign.getRhs() = sink.asExpr()) - // TODO: make sure that the parent struct of the field is not declared - // inside the switch statement. - ) + predicate isSink(DataFlow::Node sink, Field fld) { + fld.hasQualifiedName("crypto/tls", "Config", ["MinVersion", "MaxVersion"]) and + sink = fld.getAWrite().getRhs() } override predicate isSource(DataFlow::Node source) { isSource(source, _) } diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.expected b/ql/test/experimental/CWE-327/UnsafeTLS.expected index 884229e3e06..64cf3c6b019 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.expected +++ b/ql/test/experimental/CWE-327/UnsafeTLS.expected @@ -77,10 +77,6 @@ nodes | UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | semmle.label | call to InsecureCipherSuites : slice type | | UnsafeTLS.go:195:40:195:56 | implicit dereference : CipherSuite | semmle.label | implicit dereference : CipherSuite | | UnsafeTLS.go:197:25:197:36 | cipherSuites | semmle.label | cipherSuites | -| UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | -| UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | semmle.label | selection of VersionTLS10 | -| UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | semmle.label | selection of VersionTLS11 | #select | UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | UnsafeTLS.go:12:23:12:23 | 0 | Using lowest TLS version for MinVersion. | | UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | UnsafeTLS.go:21:16:21:16 | 0 | Using lowest TLS version for MinVersion. | @@ -108,7 +104,3 @@ nodes | UnsafeTLS.go:154:18:156:4 | slice literal | UnsafeTLS.go:155:5:155:45 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:154:18:156:4 | slice literal | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256. | | UnsafeTLS.go:171:25:171:94 | call to append | UnsafeTLS.go:171:53:171:93 | selection of TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 : uint16 | UnsafeTLS.go:171:25:171:94 | call to append | Use of an insecure cipher suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256. | | UnsafeTLS.go:197:25:197:36 | cipherSuites | UnsafeTLS.go:193:21:193:46 | call to InsecureCipherSuites : slice type | UnsafeTLS.go:197:25:197:36 | cipherSuites | Use of an insecure cipher suite from InsecureCipherSuites(). | -| UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | UnsafeTLS.go:207:23:207:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MaxVersion. | -| UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | UnsafeTLS.go:229:23:229:38 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | -| UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | UnsafeTLS.go:238:24:238:39 | selection of VersionTLS10 | Using insecure TLS version VersionTLS10 for MinVersion. | -| UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | UnsafeTLS.go:239:24:239:39 | selection of VersionTLS11 | Using insecure TLS version VersionTLS11 for MinVersion. | diff --git a/ql/test/experimental/CWE-327/UnsafeTLS.go b/ql/test/experimental/CWE-327/UnsafeTLS.go index e08844e003e..accbf04fa26 100644 --- a/ql/test/experimental/CWE-327/UnsafeTLS.go +++ b/ql/test/experimental/CWE-327/UnsafeTLS.go @@ -197,48 +197,3 @@ func cipherSuites() { config.CipherSuites = cipherSuites // BAD } } - -func good(version string) { - config := &tls.Config{} - - switch version { - case "1.0": - config.MinVersion = tls.VersionTLS10 // OK - config.MaxVersion = tls.VersionTLS11 // TODO: should be OK, but it's flagged as bad. - case "1.1": - config.MinVersion = tls.VersionTLS11 // OK - default: - config.MinVersion = tls.VersionTLS12 // OK - } - - _ = config -} -func badTlsVersion2(version string) { - { - config := &tls.Config{} - - switch version { - case "1.0": - config.MinVersion = tls.VersionTLS10 // OK - case "1.1": - config.MinVersion = tls.VersionTLS11 // OK - default: - config.MinVersion = tls.VersionTLS12 // OK - } - - config.MinVersion = tls.VersionTLS11 // BAD - - _ = config - } - { - - switch version { - case "1.0": - config := &tls.Config{} - config.MinVersion = tls.VersionTLS10 // BAD - config.MinVersion = tls.VersionTLS11 // BAD - _ = config - } - - } -}