diff --git a/change-notes/2020-07-22-ssh-host-checking.md b/change-notes/2020-07-22-ssh-host-checking.md new file mode 100644 index 00000000000..7f83626bf0e --- /dev/null +++ b/change-notes/2020-07-22-ssh-host-checking.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Query "Use of insecure HostKeyCallback implementation" (`go/insecure-hostkeycallback`) is promoted from experimental status. This checks for insecurely omitting SSH host-key verification. diff --git a/ql/src/experimental/CWE-322/InsecureHostKeyCallback.qhelp b/ql/src/Security/CWE-322/InsecureHostKeyCallback.qhelp similarity index 100% rename from ql/src/experimental/CWE-322/InsecureHostKeyCallback.qhelp rename to ql/src/Security/CWE-322/InsecureHostKeyCallback.qhelp diff --git a/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql b/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql new file mode 100644 index 00000000000..fbaf1f7e5f4 --- /dev/null +++ b/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql @@ -0,0 +1,112 @@ +/** + * @name Use of insecure HostKeyCallback implementation + * @description Detects insecure SSL client configurations with an implementation of the `HostKeyCallback` that accepts all host keys. + * @kind path-problem + * @problem.severity warning + * @precision high + * @id go/insecure-hostkeycallback + * @tags security + */ + +import go +import DataFlow::PathGraph + +/** The `ssh.InsecureIgnoreHostKey` function, which allows connecting to any host regardless of its host key. */ +class InsecureIgnoreHostKey extends Function { + InsecureIgnoreHostKey() { + this.hasQualifiedName("golang.org/x/crypto/ssh", "InsecureIgnoreHostKey") + } +} + +/** An SSH host-key checking function. */ +class HostKeyCallbackFunc extends DataFlow::Node { + HostKeyCallbackFunc() { + exists(NamedType nt | nt.hasQualifiedName("golang.org/x/crypto/ssh", "HostKeyCallback") | + getType().getUnderlyingType() = nt.getUnderlyingType() + ) and + // Restrict possible sources to either function definitions or + // the result of some external function call (e.g. InsecureIgnoreHostKey()) + // Otherwise we also consider typecasts, variables and other such intermediates + // as sources. + ( + this instanceof DataFlow::FunctionNode + or + exists(DataFlow::CallNode call | not exists(call.getACallee().getBody()) | + this = call.getAResult() + ) + ) + } +} + +/** A callback function value that is insecure when used as a `HostKeyCallback`, because it always returns `nil`. */ +class InsecureHostKeyCallbackFunc extends HostKeyCallbackFunc { + InsecureHostKeyCallbackFunc() { + // Either a call to InsecureIgnoreHostKey(), which we know returns an insecure callback. + this = any(InsecureIgnoreHostKey f).getACall().getAResult() + or + // Or a callback function in the source code (named or anonymous) that always returns nil. + forex(DataFlow::ResultNode returnValue | + returnValue = this.(DataFlow::FunctionNode).getAResult() + | + returnValue = Builtin::nil().getARead() + ) + } +} + +/** + * A data-flow configuration for identifying `HostKeyCallbackFunc` instances that reach `ClientConfig.HostKeyCallback` fields. + */ +class HostKeyCallbackAssignmentConfig extends DataFlow::Configuration { + HostKeyCallbackAssignmentConfig() { this = "HostKeyCallbackAssignmentConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof HostKeyCallbackFunc } + + /** + * Holds if `sink` is a value written by `write` to a field `ClientConfig.HostKeyCallback`. + */ + predicate isSink(DataFlow::Node sink, Write write) { + exists(Field f | + f.hasQualifiedName("golang.org/x/crypto/ssh", "ClientConfig", "HostKeyCallback") and + write.writesField(_, f, sink) + ) + } + + override predicate isSink(DataFlow::Node sink) { isSink(sink, _) } +} + +/** + * Holds if a secure host-check function reaches `sink` or another similar sink. + * + * A sink is considered similar if it writes to the same variable and field. + */ +predicate hostCheckReachesSink(DataFlow::PathNode sink) { + exists(HostKeyCallbackAssignmentConfig config, DataFlow::PathNode source | + not source.getNode() instanceof InsecureHostKeyCallbackFunc and + ( + config.hasFlowPath(source, sink) + or + exists( + DataFlow::PathNode otherSink, Write sinkWrite, Write otherSinkWrite, + SsaWithFields sinkAccessPath, SsaWithFields otherSinkAccessPath + | + config.hasFlowPath(source, otherSink) and + config.isSink(sink.getNode(), sinkWrite) and + config.isSink(otherSink.getNode(), otherSinkWrite) and + sinkWrite.writesField(sinkAccessPath.getAUse(), _, sink.getNode()) and + otherSinkWrite.writesField(otherSinkAccessPath.getAUse(), _, otherSink.getNode()) and + otherSinkAccessPath = sinkAccessPath.similar() + ) + ) + ) +} + +from HostKeyCallbackAssignmentConfig config, DataFlow::PathNode source, DataFlow::PathNode sink +where + config.hasFlowPath(source, sink) and + source.getNode() instanceof InsecureHostKeyCallbackFunc and + // Exclude cases where a good access-path function reaches the same or a similar sink + // (these probably indicate optional host-checking) + not hostCheckReachesSink(sink) +select sink, source, sink, + "Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@.", + source.getNode(), "this source" diff --git a/ql/src/experimental/CWE-322/InsecureHostKeyCallbackExample.go b/ql/src/Security/CWE-322/InsecureHostKeyCallbackExample.go similarity index 100% rename from ql/src/experimental/CWE-322/InsecureHostKeyCallbackExample.go rename to ql/src/Security/CWE-322/InsecureHostKeyCallbackExample.go diff --git a/ql/src/experimental/CWE-322/SecureHostKeyCallbackExample.go b/ql/src/Security/CWE-322/SecureHostKeyCallbackExample.go similarity index 100% rename from ql/src/experimental/CWE-322/SecureHostKeyCallbackExample.go rename to ql/src/Security/CWE-322/SecureHostKeyCallbackExample.go diff --git a/ql/src/experimental/CWE-322/InsecureHostKeyCallback.ql b/ql/src/experimental/CWE-322/InsecureHostKeyCallback.ql deleted file mode 100644 index 35df89e1a77..00000000000 --- a/ql/src/experimental/CWE-322/InsecureHostKeyCallback.ql +++ /dev/null @@ -1,53 +0,0 @@ -/** - * @name Use of insecure HostKeyCallback implementation - * @description Detects insecure SSL client configurations with an implementation of the `HostKeyCallback` that accepts all host keys. - * @kind path-problem - * @problem.severity error - * @id go/insecure-hostkeycallback - * @tags security - */ - -import go -import DataFlow::PathGraph - -class InsecureIgnoreHostKey extends Function { - InsecureIgnoreHostKey() { - this.hasQualifiedName("golang.org/x/crypto/ssh", "InsecureIgnoreHostKey") - } -} - -/** A callback function value that is insecure when used as a `HostKeyCallback`, because it always returns `nil`. */ -class InsecureHostKeyCallbackFunc extends DataFlow::Node { - InsecureHostKeyCallbackFunc() { - // Either a call to InsecureIgnoreHostKey(), which we know returns an insecure callback. - this = any(InsecureIgnoreHostKey f).getACall() - or - // Or a callback function in the source code (named or anonymous) that always returns nil. - forex(DataFlow::ResultNode returnValue | - returnValue = this.(DataFlow::FunctionNode).getAResult() - | - returnValue = Builtin::nil().getARead() - ) - } -} - -class HostKeyCallbackAssignmentConfig extends DataFlow::Configuration { - HostKeyCallbackAssignmentConfig() { this = "HostKeyCallbackAssignmentConfig" } - - override predicate isSource(DataFlow::Node source) { - source instanceof InsecureHostKeyCallbackFunc - } - - override predicate isSink(DataFlow::Node sink) { - exists(Field f | - f.hasQualifiedName("golang.org/x/crypto/ssh", "ClientConfig", "HostKeyCallback") and - sink = f.getAWrite().getRhs() - ) - } -} - -from HostKeyCallbackAssignmentConfig config, DataFlow::PathNode source, DataFlow::PathNode sink -where config.hasFlowPath(source, sink) -select sink, source, sink, - "Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@.", - source.getNode(), "this source" diff --git a/ql/test/experimental/CWE-322/InsecureHostKeyCallback.expected b/ql/test/experimental/CWE-322/InsecureHostKeyCallback.expected deleted file mode 100644 index 07ed8c2522b..00000000000 --- a/ql/test/experimental/CWE-322/InsecureHostKeyCallback.expected +++ /dev/null @@ -1,34 +0,0 @@ -edges -| InsecureHostKeyCallbackExample.go:12:4:14:4 | function literal : signature type | InsecureHostKeyCallbackExample.go:11:20:14:5 | type conversion | -| InsecureHostKeyCallbackExample.go:27:14:30:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:35:20:35:27 | callback | -| InsecureHostKeyCallbackExample.go:28:3:30:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:27:14:30:4 | type conversion : signature type | -| InsecureHostKeyCallbackExample.go:41:3:43:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:48:20:48:48 | type conversion | -| InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : HostKeyCallback | InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | -| InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : signature type | InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | -| InsecureHostKeyCallbackExample.go:63:22:66:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:68:35:68:50 | insecureCallback : signature type | -| InsecureHostKeyCallbackExample.go:64:3:66:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:63:22:66:4 | type conversion : signature type | -| InsecureHostKeyCallbackExample.go:68:35:68:50 | insecureCallback : signature type | InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : signature type | -| InsecureHostKeyCallbackExample.go:79:35:79:61 | call to InsecureIgnoreHostKey : HostKeyCallback | InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : HostKeyCallback | -nodes -| InsecureHostKeyCallbackExample.go:11:20:14:5 | type conversion | semmle.label | type conversion | -| InsecureHostKeyCallbackExample.go:12:4:14:4 | function literal : signature type | semmle.label | function literal : signature type | -| InsecureHostKeyCallbackExample.go:22:20:22:46 | call to InsecureIgnoreHostKey | semmle.label | call to InsecureIgnoreHostKey | -| InsecureHostKeyCallbackExample.go:27:14:30:4 | type conversion : signature type | semmle.label | type conversion : signature type | -| InsecureHostKeyCallbackExample.go:28:3:30:3 | function literal : signature type | semmle.label | function literal : signature type | -| InsecureHostKeyCallbackExample.go:35:20:35:27 | callback | semmle.label | callback | -| InsecureHostKeyCallbackExample.go:41:3:43:3 | function literal : signature type | semmle.label | function literal : signature type | -| InsecureHostKeyCallbackExample.go:48:20:48:48 | type conversion | semmle.label | type conversion | -| InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : HostKeyCallback | semmle.label | definition of callback : HostKeyCallback | -| InsecureHostKeyCallbackExample.go:52:39:52:46 | definition of callback : signature type | semmle.label | definition of callback : signature type | -| InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | semmle.label | callback | -| InsecureHostKeyCallbackExample.go:63:22:66:4 | type conversion : signature type | semmle.label | type conversion : signature type | -| InsecureHostKeyCallbackExample.go:64:3:66:3 | function literal : signature type | semmle.label | function literal : signature type | -| InsecureHostKeyCallbackExample.go:68:35:68:50 | insecureCallback : signature type | semmle.label | insecureCallback : signature type | -| InsecureHostKeyCallbackExample.go:79:35:79:61 | call to InsecureIgnoreHostKey : HostKeyCallback | semmle.label | call to InsecureIgnoreHostKey : HostKeyCallback | -#select -| InsecureHostKeyCallbackExample.go:11:20:14:5 | type conversion | InsecureHostKeyCallbackExample.go:12:4:14:4 | function literal : signature type | InsecureHostKeyCallbackExample.go:11:20:14:5 | type conversion | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:12:4:14:4 | function literal | this source | -| InsecureHostKeyCallbackExample.go:22:20:22:46 | call to InsecureIgnoreHostKey | InsecureHostKeyCallbackExample.go:22:20:22:46 | call to InsecureIgnoreHostKey | InsecureHostKeyCallbackExample.go:22:20:22:46 | call to InsecureIgnoreHostKey | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:22:20:22:46 | call to InsecureIgnoreHostKey | this source | -| InsecureHostKeyCallbackExample.go:35:20:35:27 | callback | InsecureHostKeyCallbackExample.go:28:3:30:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:35:20:35:27 | callback | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:28:3:30:3 | function literal | this source | -| InsecureHostKeyCallbackExample.go:48:20:48:48 | type conversion | InsecureHostKeyCallbackExample.go:41:3:43:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:48:20:48:48 | type conversion | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:41:3:43:3 | function literal | this source | -| InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | InsecureHostKeyCallbackExample.go:64:3:66:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:64:3:66:3 | function literal | this source | -| InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | InsecureHostKeyCallbackExample.go:79:35:79:61 | call to InsecureIgnoreHostKey : HostKeyCallback | InsecureHostKeyCallbackExample.go:56:20:56:27 | callback | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:79:35:79:61 | call to InsecureIgnoreHostKey | this source | diff --git a/ql/test/experimental/CWE-322/InsecureHostKeyCallback.qlref b/ql/test/experimental/CWE-322/InsecureHostKeyCallback.qlref deleted file mode 100644 index 006d685c747..00000000000 --- a/ql/test/experimental/CWE-322/InsecureHostKeyCallback.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/CWE-322/InsecureHostKeyCallback.ql \ No newline at end of file diff --git a/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.expected b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.expected new file mode 100644 index 00000000000..0e851d743b2 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.expected @@ -0,0 +1,49 @@ +edges +| InsecureHostKeyCallbackExample.go:16:4:18:4 | function literal : signature type | InsecureHostKeyCallbackExample.go:15:20:18:5 | type conversion | +| InsecureHostKeyCallbackExample.go:31:14:34:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:39:20:39:27 | callback | +| InsecureHostKeyCallbackExample.go:32:3:34:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:31:14:34:4 | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:45:3:47:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | +| InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : HostKeyCallback | InsecureHostKeyCallbackExample.go:62:20:62:27 | callback | +| InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | InsecureHostKeyCallbackExample.go:62:20:62:27 | callback | +| InsecureHostKeyCallbackExample.go:68:48:68:55 | definition of callback : signature type | InsecureHostKeyCallbackExample.go:78:28:78:35 | callback | +| InsecureHostKeyCallbackExample.go:94:3:94:45 | ... := ...[0] : HostKeyCallback | InsecureHostKeyCallbackExample.go:95:28:95:35 | callback | +| InsecureHostKeyCallbackExample.go:102:22:105:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:107:35:107:50 | insecureCallback : signature type | +| InsecureHostKeyCallbackExample.go:103:3:105:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:102:22:105:4 | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:107:35:107:50 | insecureCallback : signature type | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | +| InsecureHostKeyCallbackExample.go:109:31:115:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:117:35:117:59 | potentiallySecureCallback : signature type | +| InsecureHostKeyCallbackExample.go:109:31:115:4 | type conversion : signature type | InsecureHostKeyCallbackExample.go:120:44:120:68 | potentiallySecureCallback : signature type | +| InsecureHostKeyCallbackExample.go:110:3:115:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:109:31:115:4 | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:117:35:117:59 | potentiallySecureCallback : signature type | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | +| InsecureHostKeyCallbackExample.go:118:35:118:61 | call to InsecureIgnoreHostKey : HostKeyCallback | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : HostKeyCallback | +| InsecureHostKeyCallbackExample.go:120:44:120:68 | potentiallySecureCallback : signature type | InsecureHostKeyCallbackExample.go:68:48:68:55 | definition of callback : signature type | +nodes +| InsecureHostKeyCallbackExample.go:15:20:18:5 | type conversion | semmle.label | type conversion | +| InsecureHostKeyCallbackExample.go:16:4:18:4 | function literal : signature type | semmle.label | function literal : signature type | +| InsecureHostKeyCallbackExample.go:26:20:26:46 | call to InsecureIgnoreHostKey | semmle.label | call to InsecureIgnoreHostKey | +| InsecureHostKeyCallbackExample.go:31:14:34:4 | type conversion : signature type | semmle.label | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:32:3:34:3 | function literal : signature type | semmle.label | function literal : signature type | +| InsecureHostKeyCallbackExample.go:39:20:39:27 | callback | semmle.label | callback | +| InsecureHostKeyCallbackExample.go:45:3:47:3 | function literal : signature type | semmle.label | function literal : signature type | +| InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | semmle.label | type conversion | +| InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : HostKeyCallback | semmle.label | definition of callback : HostKeyCallback | +| InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | semmle.label | definition of callback : signature type | +| InsecureHostKeyCallbackExample.go:62:20:62:27 | callback | semmle.label | callback | +| InsecureHostKeyCallbackExample.go:68:48:68:55 | definition of callback : signature type | semmle.label | definition of callback : signature type | +| InsecureHostKeyCallbackExample.go:76:28:76:54 | call to InsecureIgnoreHostKey | semmle.label | call to InsecureIgnoreHostKey | +| InsecureHostKeyCallbackExample.go:78:28:78:35 | callback | semmle.label | callback | +| InsecureHostKeyCallbackExample.go:92:28:92:54 | call to InsecureIgnoreHostKey | semmle.label | call to InsecureIgnoreHostKey | +| InsecureHostKeyCallbackExample.go:94:3:94:45 | ... := ...[0] : HostKeyCallback | semmle.label | ... := ...[0] : HostKeyCallback | +| InsecureHostKeyCallbackExample.go:95:28:95:35 | callback | semmle.label | callback | +| InsecureHostKeyCallbackExample.go:102:22:105:4 | type conversion : signature type | semmle.label | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:103:3:105:3 | function literal : signature type | semmle.label | function literal : signature type | +| InsecureHostKeyCallbackExample.go:107:35:107:50 | insecureCallback : signature type | semmle.label | insecureCallback : signature type | +| InsecureHostKeyCallbackExample.go:109:31:115:4 | type conversion : signature type | semmle.label | type conversion : signature type | +| InsecureHostKeyCallbackExample.go:110:3:115:3 | function literal : signature type | semmle.label | function literal : signature type | +| InsecureHostKeyCallbackExample.go:117:35:117:59 | potentiallySecureCallback : signature type | semmle.label | potentiallySecureCallback : signature type | +| InsecureHostKeyCallbackExample.go:118:35:118:61 | call to InsecureIgnoreHostKey : HostKeyCallback | semmle.label | call to InsecureIgnoreHostKey : HostKeyCallback | +| InsecureHostKeyCallbackExample.go:120:44:120:68 | potentiallySecureCallback : signature type | semmle.label | potentiallySecureCallback : signature type | +#select +| InsecureHostKeyCallbackExample.go:15:20:18:5 | type conversion | InsecureHostKeyCallbackExample.go:16:4:18:4 | function literal : signature type | InsecureHostKeyCallbackExample.go:15:20:18:5 | type conversion | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:16:4:18:4 | function literal | this source | +| InsecureHostKeyCallbackExample.go:26:20:26:46 | call to InsecureIgnoreHostKey | InsecureHostKeyCallbackExample.go:26:20:26:46 | call to InsecureIgnoreHostKey | InsecureHostKeyCallbackExample.go:26:20:26:46 | call to InsecureIgnoreHostKey | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:26:20:26:46 | call to InsecureIgnoreHostKey | this source | +| InsecureHostKeyCallbackExample.go:39:20:39:27 | callback | InsecureHostKeyCallbackExample.go:32:3:34:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:39:20:39:27 | callback | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:32:3:34:3 | function literal | this source | +| InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | InsecureHostKeyCallbackExample.go:45:3:47:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@. | InsecureHostKeyCallbackExample.go:45:3:47:3 | function literal | this source | diff --git a/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.qlref b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.qlref new file mode 100644 index 00000000000..b5f8712594d --- /dev/null +++ b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.qlref @@ -0,0 +1 @@ +Security/CWE-322/InsecureHostKeyCallback.ql diff --git a/ql/test/experimental/CWE-322/InsecureHostKeyCallbackExample.go b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallbackExample.go similarity index 51% rename from ql/test/experimental/CWE-322/InsecureHostKeyCallbackExample.go rename to ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallbackExample.go index f9870e4937e..0c7f9466488 100644 --- a/ql/test/experimental/CWE-322/InsecureHostKeyCallbackExample.go +++ b/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallbackExample.go @@ -1,14 +1,18 @@ package main -import "net" -import "fmt" -import "golang.org/x/crypto/ssh" +import ( + "fmt" + "net" + + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" +) func insecureSSHClientConfig() { _ = &ssh.ClientConfig{ User: "user", Auth: []ssh.AuthMethod{nil}, - HostKeyCallback: ssh.HostKeyCallback( + HostKeyCallback: ssh.HostKeyCallback( // BAD func(hostname string, remote net.Addr, key ssh.PublicKey) error { return nil }), @@ -19,7 +23,7 @@ func insecureSSHClientConfigAlt() { _ = &ssh.ClientConfig{ User: "user", Auth: []ssh.AuthMethod{nil}, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), + HostKeyCallback: ssh.InsecureIgnoreHostKey(), // BAD } } @@ -32,7 +36,7 @@ func insecureSSHClientConfigLocalFlow() { _ = &ssh.ClientConfig{ User: "user", Auth: []ssh.AuthMethod{nil}, - HostKeyCallback: callback, + HostKeyCallback: callback, // BAD } } @@ -45,15 +49,50 @@ func insecureSSHClientConfigLocalFlowAlt() { _ = &ssh.ClientConfig{ User: "user", Auth: []ssh.AuthMethod{nil}, - HostKeyCallback: ssh.HostKeyCallback(callback), + HostKeyCallback: ssh.HostKeyCallback(callback), // BAD } } +// Check that insecure and secure functions flowing together to the same +// sink is not flagged (we assume this is configurable security) func potentialInsecureSSHClientConfig(callback ssh.HostKeyCallback) { _ = &ssh.ClientConfig{ User: "user", Auth: []ssh.AuthMethod{nil}, - HostKeyCallback: callback, + HostKeyCallback: callback, // OK + } +} + +// Check that insecure and secure functions flowing to different writes to +// the same objects are not flagged (we assume this is configurable security) +func potentialInsecureSSHClientConfigTwoWrites(callback ssh.HostKeyCallback) { + config := &ssh.ClientConfig{ + User: "user", + Auth: []ssh.AuthMethod{nil}, + HostKeyCallback: nil, + } + + if callback == nil { + config.HostKeyCallback = ssh.InsecureIgnoreHostKey() // OK + } else { + config.HostKeyCallback = callback + } +} + +// Check that insecure and secure functions flowing to different writes to +// the same objects are not flagged (we assume this is configurable security) +func potentialInsecureSSHClientConfigUsingKnownHosts(x bool) { + config := &ssh.ClientConfig{ + User: "user", + Auth: []ssh.AuthMethod{nil}, + HostKeyCallback: nil, + } + + if x { + config.HostKeyCallback = ssh.InsecureIgnoreHostKey() // OK + } else { + callback, err := knownhosts.New("somefile") + config.HostKeyCallback = callback } } @@ -77,4 +116,6 @@ func main() { potentialInsecureSSHClientConfig(potentiallySecureCallback) potentialInsecureSSHClientConfig(ssh.InsecureIgnoreHostKey()) + + potentialInsecureSSHClientConfigTwoWrites(potentiallySecureCallback) } diff --git a/ql/test/experimental/CWE-322/go.mod b/ql/test/query-tests/Security/CWE-322/go.mod similarity index 100% rename from ql/test/experimental/CWE-322/go.mod rename to ql/test/query-tests/Security/CWE-322/go.mod diff --git a/ql/test/experimental/CWE-322/vendor/golang.org/LICENSE b/ql/test/query-tests/Security/CWE-322/vendor/golang.org/LICENSE similarity index 100% rename from ql/test/experimental/CWE-322/vendor/golang.org/LICENSE rename to ql/test/query-tests/Security/CWE-322/vendor/golang.org/LICENSE diff --git a/ql/test/query-tests/Security/CWE-322/vendor/golang.org/x/crypto/ssh/knownhosts/stub.go b/ql/test/query-tests/Security/CWE-322/vendor/golang.org/x/crypto/ssh/knownhosts/stub.go new file mode 100644 index 00000000000..374e3e83356 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-322/vendor/golang.org/x/crypto/ssh/knownhosts/stub.go @@ -0,0 +1,12 @@ +// A simple manual stub of golang.org/x/crypto/ssh/knownhosts.New +// See the LICENSE file for information about the licensing of the original library. + +package knownhosts + +import ( + "golang.org/x/crypto/ssh" +) + +func New(files ...string) (ssh.HostKeyCallback, error) { + return nil, nil +} diff --git a/ql/test/experimental/CWE-322/vendor/golang.org/x/crypto/ssh/stub.go b/ql/test/query-tests/Security/CWE-322/vendor/golang.org/x/crypto/ssh/stub.go similarity index 100% rename from ql/test/experimental/CWE-322/vendor/golang.org/x/crypto/ssh/stub.go rename to ql/test/query-tests/Security/CWE-322/vendor/golang.org/x/crypto/ssh/stub.go diff --git a/ql/test/experimental/CWE-322/vendor/modules.txt b/ql/test/query-tests/Security/CWE-322/vendor/modules.txt similarity index 100% rename from ql/test/experimental/CWE-322/vendor/modules.txt rename to ql/test/query-tests/Security/CWE-322/vendor/modules.txt