Severity and mitigation

Like I said in my previous blog post about a vulnerability in libssh2: don’t panic! This is not a vulnerability in openssh, so it doesn’t affect the ssh that we all use every day. libssh2 is a client-side C library, which enables applications to connect to an SSH server. This also isn’t a vulnerability in libssh, which is an unrelated C library which provides similar functionality to libssh2.

The vulnerability exists in libssh2 version 1.9.0 and earlier. At the time of writing, the bug has been fixed on master, but a new official version including the fix has not yet been released.

The vulnerability is an out-of-bounds read, potentially leading to either denial of service or remote information disclosure. It is triggered when libssh2 is used to connect to a malicious SSH server. The overflow occurs when the SSH server sends a disconnect message, which means that the vulnerability can be triggered early in the connection process, before authentication is completed.

Triggering the vulnerability

The source location of the vulnerability is packet.c:480:

if(message_len < datalen-13) {

The value of datalen is untrusted because it is controlled by the remote SSH server. If datalen == 11, for example, then the subtraction will overflow and the bounds check of message_len is ineffective. message_len is a 32-bit unsigned integer that is also controlled by the remote SSH server, so this can lead to an out-of-bounds read on line 485:

language_len =
    _libssh2_ntohu32(data + 9 + message_len);

The out-of-bounds read will usually just cause a segmentation fault, but there is also potential for it to cause other kinds of harm in the call to LIBSSH2_DISCONNECT on line 499:

if(session->ssh_msg_disconnect) {
    LIBSSH2_DISCONNECT(session, reason, message,
                       message_len, language, language_len);
}

It depends on how the libssh2 library is used, because session->ssh_msg_disconnect is a callback function that is null by default, but can be set by the user of the library (by calling libssh2_session_callback_set).

I have written a proof-of-concept exploit in which a malicious SSH server returns a disconnect message with datalen == 11 and message_len == 0x41414141, which causes libssh2 to crash with a segmentation fault.

Variant analysis on integer overflows in libssh2

I wrote my last blog post about libssh2 toward the end of June 2019. To get the technical details accurate for the blog post, I needed to go back and take another look at the libssh2 code. As I did so, I started to notice more bugs. In particular, I noticed a lot of careless bounds-checking code like the bug described above.

The goal of the query that I wrote for this vulnerability was slightly different than what we normally aim for when we’re doing variant analysis. Usually, variant analysis is about finding all variants of a bug and, if possible, creating a query that is reusable across multiple codebases. But my goal was more narrowly focussed on libssh2 and the specific vulnerability that I had discovered.

When I report a security vulnerability to a vendor, I usually try to include two things in my report:¹

1. A proof of concept exploit for one of the bugs.
2. A CodeQL query that identifies all the code locations that I think should be fixed.

There are several benefits of including a CodeQL query as well as a PoC:

1. If the code contains several very similar bugs, then I can write a query that enumerates them all.
2. The query enables me to easily check whether the bugs have been fixed (which is very convenient when the 90 day deadline is about to expire).
3. I can include the CodeQL query and its list of results as a single URL, which is convenient for me and hopefully also for the recipient.

Creating a PoC is often quite a lot of work, so if there are multiple very similar bugs then I usually only write a PoC for one of them. My feeling is that one PoC is sufficient to prove that the security impact is real, provided that the other variants are clearly very similar. The query below is tuned for this use-case. The goal of this query is not to find all the integer overflow vulnerabilities in libssh2, and it also won’t scale to other codebases due to some libssh2-specific details encoded in the query. (Scaling across multiple codebases is normally one of the main goals of variant analysis) Rather, its goal is to find the bug that my PoC triggers plus the other close variants.

/**
 * @kind path-problem
 */

import cpp
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph

class Config extends DataFlow::Configuration {
  Config() { this = "_libssh2_ntohl bounds check overflow" }

  override predicate isSource(DataFlow::Node source) {
    source.asExpr().(FunctionCall).getTarget().getName().matches("_libssh2_ntoh%")
  }

  override predicate isSink(DataFlow::Node sink) {
    convertedExprMightOverflowNegatively(sink.asExpr()) and
    exists(RelationalOperation cmp | cmp.getAnOperand() = sink.asExpr())
  }

  override predicate isAdditionalFlowStep(DataFlow::Node source,
                                          DataFlow::Node target) {
    exists(Field f |
      source.asExpr() = f.getAnAssignedValue() and
      target.asExpr() = f.getAnAccess())
    or
    target.asExpr().(AddExpr).getAnOperand() = source.asExpr()
    or
    target.asExpr().(SubExpr).getAnOperand() = source.asExpr()
  }
}

from Config cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink,
  "possible integer overflow of tainted expression in bounds check"

This query is very similar to the query that my colleague Fermín Serna wrote for the U-Boot NFS vulnerabilities. The isSource predicate finds calls to _libssh2_ntohu32 and _libssh2_ntohu64, which are used for network to host byte order translation. These functions are often excellent proxies for “attacker-controlled data”. But my isSink predicate is completely different from Fermín’s. It looks for comparison operations that contain a sub-expression that might overflow. For example, message_len < datalen-13 is a comparison expression in which the sub-expression datalen-13 might overflow. My query also overrides the optional isAdditionalFlowStep predicate to customize the set of data-flow edges. I tuned this predicate to produce an accurate list of close variants.

I sent the query results to the libssh2 team when I reported the bug on July 1, 2019. Most of those results have since been fixed, but you can see from the second link that some new results have also appeared. I have not investigated whether any of the new results are exploitable.

Timeline

• July 1, 2019: I privately disclose the vulnerability to the libssh2 team.
• July 2, 2019: The libssh2 team send an initial reply.
• August 30, 2019: The libssh2 team fix the bug on their development branch.
• October 10, 2019: I notify the libssh2 team that our 90 day disclosure deadline has expired and ask when they are planning to release a new version.
• October 11, 2019: CVE-2019-17498 assigned.
• October 15, 2019: I publicly disclose CVE-2019-17498 by posting my PoC on GitHub.