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 - } - - } -}