Re: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 5, 2018, 20:43 UTC
Message-ID: <20180205204312.GB104086@aiede.svl.corp.google.com>
In-Reply-To: <151785928011.30076.5964248840190566119.reportbug@eldamar.local>

+cc: upstream Hi,

Salvatore Bonaccorso wrote[1]:

Show 18 quoted lines

> the following vulnerability was published for git.
>
> CVE-2018-1000021[0]:
> |client prints server sent ANSI escape codes to the terminal, allowing
> |for unverified messages to potentially execute arbitrary commands
>
> Creating this bug to track the issue in the BTS. Apparently the CVE
> was sssigned without notifying/discussing it with upstream, at least
> according to [1].
>
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
>
> For further information see:
>
> [0] https://security-tracker.debian.org/tracker/CVE-2018-1000021
>     https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000021
> [1] https://bugzilla.novell.com/show_bug.cgi?id=1079389#c1

Thanks. Upstream was notified about this and we dropped the ball on passing it on to a more public forum. Sorry about that.

I'd be interested in your advice on this. There are cases where the user may *want* ANSI escape codes to be passed through without change and other cases where the user doesn't want that. Commands like "git diff" pass their output through a pager by default, which itself may or may not sanitize the output.

In other words, there are multiple components at play:

 1. A terminal.  IMHO, it is completely inexcusable these days for a
    terminal to allow arbitrary code execution by writing output to
    it.  If bugs of that kind still exist, I think we should fix them
    (and perhaps even make it a requirement in Debian policy to make
    the expectations clear for new terminals).

    That said, for defense in depth, it can be useful to also guard
    against this kind of issue in other components.  In particular:

 2. A pager.  Are there clear guidelines for what it is safe and not
    safe for a pager to write to a terminal?

    "less -R" tries to only allow ANSI "color" escape sequences
    through but I wouldn't be surprised if there are some cases it
    misses.

 3. Output formats.  Some git commands are designed for scripting
    and do not have a sensible way to sanitize their output without
    breaking scripts.  Fortunately, in the case of "git diff", git
    has a notion of a "binary patch" where everything is sanitized,
    at the cost of the output being unreadable to a human (email-safe
    characters but not something that a human can read at a glance).
    So if we know what sequences to avoid writing to stdout, then we
    can treat files with those sequences as binary.

Pointers welcome.

Thanks, Jonathan

[1] https://bugs.debian.org/889680