|
| 1 | +/** |
| 2 | + * @name Use of insecure HostKeyCallback implementation |
| 3 | + * @description Detects insecure SSL client configurations with an implementation of the `HostKeyCallback` that accepts all host keys. |
| 4 | + * @kind path-problem |
| 5 | + * @problem.severity warning |
| 6 | + * @precision high |
| 7 | + * @id go/insecure-hostkeycallback |
| 8 | + * @tags security |
| 9 | + */ |
| 10 | + |
| 11 | +import go |
| 12 | +import DataFlow::PathGraph |
| 13 | + |
| 14 | +/** The `ssh.InsecureIgnoreHostKey` function, which allows connecting to any host regardless of its host key. */ |
| 15 | +class InsecureIgnoreHostKey extends Function { |
| 16 | + InsecureIgnoreHostKey() { |
| 17 | + this.hasQualifiedName("golang.org/x/crypto/ssh", "InsecureIgnoreHostKey") |
| 18 | + } |
| 19 | +} |
| 20 | + |
| 21 | +/** An SSH host-key checking function. */ |
| 22 | +class HostKeyCallbackFunc extends DataFlow::Node { |
| 23 | + HostKeyCallbackFunc() { |
| 24 | + exists(NamedType nt | nt.hasQualifiedName("golang.org/x/crypto/ssh", "HostKeyCallback") | |
| 25 | + getType().getUnderlyingType() = nt.getUnderlyingType() |
| 26 | + ) and |
| 27 | + // Restrict possible sources to either function definitions or |
| 28 | + // the result of some external function call (e.g. InsecureIgnoreHostKey()) |
| 29 | + // Otherwise we also consider typecasts, variables and other such intermediates |
| 30 | + // as sources. |
| 31 | + ( |
| 32 | + this instanceof DataFlow::FunctionNode |
| 33 | + or |
| 34 | + exists(DataFlow::CallNode call | not exists(call.getACallee().getBody()) | |
| 35 | + this = call.getAResult() |
| 36 | + ) |
| 37 | + ) |
| 38 | + } |
| 39 | +} |
| 40 | + |
| 41 | +/** A callback function value that is insecure when used as a `HostKeyCallback`, because it always returns `nil`. */ |
| 42 | +class InsecureHostKeyCallbackFunc extends HostKeyCallbackFunc { |
| 43 | + InsecureHostKeyCallbackFunc() { |
| 44 | + // Either a call to InsecureIgnoreHostKey(), which we know returns an insecure callback. |
| 45 | + this = any(InsecureIgnoreHostKey f).getACall().getAResult() |
| 46 | + or |
| 47 | + // Or a callback function in the source code (named or anonymous) that always returns nil. |
| 48 | + forex(DataFlow::ResultNode returnValue | |
| 49 | + returnValue = this.(DataFlow::FunctionNode).getAResult() |
| 50 | + | |
| 51 | + returnValue = Builtin::nil().getARead() |
| 52 | + ) |
| 53 | + } |
| 54 | +} |
| 55 | + |
| 56 | +/** |
| 57 | + * A data-flow configuration for identifying `HostKeyCallbackFunc` instances that reach `ClientConfig.HostKeyCallback` fields. |
| 58 | + */ |
| 59 | +class HostKeyCallbackAssignmentConfig extends DataFlow::Configuration { |
| 60 | + HostKeyCallbackAssignmentConfig() { this = "HostKeyCallbackAssignmentConfig" } |
| 61 | + |
| 62 | + override predicate isSource(DataFlow::Node source) { source instanceof HostKeyCallbackFunc } |
| 63 | + |
| 64 | + /** |
| 65 | + * Holds if `sink` is a value written by `write` to a field `ClientConfig.HostKeyCallback`. |
| 66 | + */ |
| 67 | + predicate isSink(DataFlow::Node sink, Write write) { |
| 68 | + exists(Field f | |
| 69 | + f.hasQualifiedName("golang.org/x/crypto/ssh", "ClientConfig", "HostKeyCallback") and |
| 70 | + write.writesField(_, f, sink) |
| 71 | + ) |
| 72 | + } |
| 73 | + |
| 74 | + override predicate isSink(DataFlow::Node sink) { isSink(sink, _) } |
| 75 | +} |
| 76 | + |
| 77 | +/** |
| 78 | + * Holds if a secure host-check function reaches `sink` or another similar sink. |
| 79 | + * |
| 80 | + * A sink is considered similar if it writes to the same variable and field. |
| 81 | + */ |
| 82 | +predicate hostCheckReachesSink(DataFlow::PathNode sink) { |
| 83 | + exists(HostKeyCallbackAssignmentConfig config, DataFlow::PathNode source | |
| 84 | + not source.getNode() instanceof InsecureHostKeyCallbackFunc and |
| 85 | + ( |
| 86 | + config.hasFlowPath(source, sink) |
| 87 | + or |
| 88 | + exists( |
| 89 | + DataFlow::PathNode otherSink, Write sinkWrite, Write otherSinkWrite, |
| 90 | + SsaWithFields sinkAccessPath, SsaWithFields otherSinkAccessPath |
| 91 | + | |
| 92 | + config.hasFlowPath(source, otherSink) and |
| 93 | + config.isSink(sink.getNode(), sinkWrite) and |
| 94 | + config.isSink(otherSink.getNode(), otherSinkWrite) and |
| 95 | + sinkWrite.writesField(sinkAccessPath.getAUse(), _, sink.getNode()) and |
| 96 | + otherSinkWrite.writesField(otherSinkAccessPath.getAUse(), _, otherSink.getNode()) and |
| 97 | + otherSinkAccessPath = sinkAccessPath.similar() |
| 98 | + ) |
| 99 | + ) |
| 100 | + ) |
| 101 | +} |
| 102 | + |
| 103 | +from HostKeyCallbackAssignmentConfig config, DataFlow::PathNode source, DataFlow::PathNode sink |
| 104 | +where |
| 105 | + config.hasFlowPath(source, sink) and |
| 106 | + source.getNode() instanceof InsecureHostKeyCallbackFunc and |
| 107 | + // Exclude cases where a good access-path function reaches the same or a similar sink |
| 108 | + // (these probably indicate optional host-checking) |
| 109 | + not hostCheckReachesSink(sink) |
| 110 | +select sink, source, sink, |
| 111 | + "Configuring SSH ClientConfig with insecure HostKeyCallback implementation from $@.", |
| 112 | + source.getNode(), "this source" |
0 commit comments