[PATCH v4 0/6] Sanitize sideband channel messages

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

Git's sideband channel passes server output directly to the client terminal
without sanitizing it, creating an ANSI escape sequence injection
vulnerability (CWE-150 [https://cwe.mitre.org/data/definitions/150.html]). A
malicious or compromised server can corrupt terminal state, obscure
information, or inject characters into the terminal's input buffer (which
the terminal will interpret as if the user had typed them).

Git users have no mechanism to distinguish between Git's legitimate output
and content displayed (or hidden) via attack sequences.

This series aims to fix the vulnerability by sanitizing control characters
in the sideband output. To address concerns about existing hacks that
exploit Git's lack of sanitizing, this security fix will only kick in with
Git v3.0, where the default will change to pass ANSI color sequences (SGR
codes) through by default, since server-side hooks exist that use these for
visibility (e.g. https://github.com/kikeonline/githook-explode). By default,
all other control characters will be rendered in caret notation (e.g., ESC
becomes ^[).

Users who need different behavior get configuration options:
sideband.allowControlCharacters provides an escape hatch for environments
that require raw passthrough. The defaults in Git v3.0 will be secure.

This series applies cleanly on v2.53.0.

Changes since v3:

 * Targeting maint-2.53 now instead of maint-2.47.
 * Turned the "safe by default" behavior into a breaking change that will be
   in effect in Git v3.0.

Changes since v2:

 * Added curly brackets around a single-line if clause.
 * Enclosed the values in the documentation within backticks.
 * Aligned the enum values for better readability.
 * Added support for sideband.<url>.allowControlCharacters (à la
   http.<url>.*) on top of sideband.allowControlCharacters.

Changes since v1:

 * Applied the suggestions by Phillip and brian.
 * Rebased onto v2.47.3.
 * Added more categories of ANSI Escape sequences that can be enabled (but
   that are off by default because they could be used to hide information).

Johannes Schindelin (6):
  sideband: mask control characters
  sideband: introduce an "escape hatch" to allow control characters
  sideband: do allow ANSI color sequences by default
  sideband: add options to allow more control sequences to be passed
    through
  sideband: offer to configure sanitizing on a per-URL basis
  sideband: delay sanitizing by default to Git v3.0

 Documentation/config.adoc           |   2 +
 Documentation/config/sideband.adoc  |  39 ++++++
 sideband.c                          | 185 +++++++++++++++++++++++++++-
 sideband.h                          |  14 +++
 t/t5409-colorize-remote-messages.sh | 100 +++++++++++++++
 transport.c                         |   3 +
 6 files changed, 341 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/config/sideband.adoc


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1853%2Fdscho%2Fsanitize-sideband-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1853/dscho/sanitize-sideband-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1853

Range-diff vs v3:

 1:  e6b71af0ca = 1:  7addb9ca52 sideband: mask control characters
 2:  8f64d65844 ! 2:  20058534e8 sideband: introduce an "escape hatch" to allow control characters
     @@ Commit message
          Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     - ## Documentation/config.txt ##
     -@@ Documentation/config.txt: include::config/sequencer.txt[]
     + ## Documentation/config.adoc ##
     +@@ Documentation/config.adoc: include::config/sequencer.adoc[]
       
     - include::config/showbranch.txt[]
     + include::config/showbranch.adoc[]
       
     -+include::config/sideband.txt[]
     ++include::config/sideband.adoc[]
      +
     - include::config/sparse.txt[]
     + include::config/sparse.adoc[]
       
     - include::config/splitindex.txt[]
     + include::config/splitindex.adoc[]
      
     - ## Documentation/config/sideband.txt (new) ##
     + ## Documentation/config/sideband.adoc (new) ##
      @@
      +sideband.allowControlCharacters::
      +	By default, control characters that are delivered via the sideband
     @@ sideband.c: static struct keyword_entry keywords[] = {
      +static int allow_control_characters;
      +
       /* Returns a color setting (GIT_COLOR_NEVER, etc). */
     - static int use_sideband_colors(void)
     + static enum git_colorbool use_sideband_colors(void)
       {
     -@@ sideband.c: static int use_sideband_colors(void)
     - 	if (use_sideband_colors_cached >= 0)
     +@@ sideband.c: static enum git_colorbool use_sideband_colors(void)
     + 	if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
       		return use_sideband_colors_cached;
       
     -+	git_config_get_bool("sideband.allowcontrolcharacters",
     ++	repo_config_get_bool(the_repository, "sideband.allowcontrolcharacters",
      +			    &allow_control_characters);
      +
     - 	if (!git_config_get_string_tmp(key, &value))
     + 	if (!repo_config_get_string_tmp(the_repository, key, &value))
       		use_sideband_colors_cached = git_config_colorbool(key, value);
     - 	else if (!git_config_get_string_tmp("color.ui", &value))
     + 	else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value))
      @@ sideband.c: void list_config_color_sideband_slots(struct string_list *list, const char *pref
       
       static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
 3:  44838acacc ! 3:  919111f590 sideband: do allow ANSI color sequences by default
     @@ Commit message
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     - ## Documentation/config/sideband.txt ##
     + ## Documentation/config/sideband.adoc ##
      @@
       sideband.allowControlCharacters::
       	By default, control characters that are delivered via the sideband
     @@ sideband.c: static struct keyword_entry keywords[] = {
      +} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
       
       /* Returns a color setting (GIT_COLOR_NEVER, etc). */
     - static int use_sideband_colors(void)
     -@@ sideband.c: static int use_sideband_colors(void)
     - 	if (use_sideband_colors_cached >= 0)
     + static enum git_colorbool use_sideband_colors(void)
     +@@ sideband.c: static enum git_colorbool use_sideband_colors(void)
     + 	if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
       		return use_sideband_colors_cached;
       
     --	git_config_get_bool("sideband.allowcontrolcharacters",
     +-	repo_config_get_bool(the_repository, "sideband.allowcontrolcharacters",
      -			    &allow_control_characters);
     -+	switch (git_config_get_maybe_bool("sideband.allowcontrolcharacters", &i)) {
     ++	switch (repo_config_get_maybe_bool(the_repository, "sideband.allowcontrolcharacters", &i)) {
      +	case 0: /* Boolean value */
      +		allow_control_characters = i ? ALLOW_ALL_CONTROL_CHARACTERS :
      +			ALLOW_NO_CONTROL_CHARACTERS;
      +		break;
      +	case -1: /* non-Boolean value */
     -+		if (git_config_get_string_tmp("sideband.allowcontrolcharacters",
     ++		if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters",
      +					      &value))
      +			; /* huh? `get_maybe_bool()` returned -1 */
      +		else if (!strcmp(value, "default"))
     @@ sideband.c: static int use_sideband_colors(void)
      +		break; /* not configured */
      +	}
       
     - 	if (!git_config_get_string_tmp(key, &value))
     + 	if (!repo_config_get_string_tmp(the_repository, key, &value))
       		use_sideband_colors_cached = git_config_colorbool(key, value);
      @@ sideband.c: void list_config_color_sideband_slots(struct string_list *list, const char *pref
       		list_config_item(list, prefix, keywords[i].keyword);
 4:  cc578465b9 ! 4:  ec48f1cba1 sideband: add options to allow more control sequences to be passed through
     @@ Commit message
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     - ## Documentation/config/sideband.txt ##
     -@@ Documentation/config/sideband.txt: sideband.allowControlCharacters::
     + ## Documentation/config/sideband.adoc ##
     +@@ Documentation/config/sideband.adoc: sideband.allowControlCharacters::
       	By default, control characters that are delivered via the sideband
       	are masked, except ANSI color sequences. This prevents potentially
       	unwanted ANSI escape sequences from being sent to the terminal. Use
     @@ sideband.c: static struct keyword_entry keywords[] = {
      +}
       
       /* Returns a color setting (GIT_COLOR_NEVER, etc). */
     - static int use_sideband_colors(void)
     -@@ sideband.c: static int use_sideband_colors(void)
     - 		if (git_config_get_string_tmp("sideband.allowcontrolcharacters",
     + static enum git_colorbool use_sideband_colors(void)
     +@@ sideband.c: static enum git_colorbool use_sideband_colors(void)
     + 		if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters",
       					      &value))
       			; /* huh? `get_maybe_bool()` returned -1 */
      -		else if (!strcmp(value, "default"))
 5:  f2eb0a758c ! 5:  692d1a63ed sideband: offer to configure sanitizing on a per-URL basis
     @@ Commit message
          Suggested-by: Junio Hamano <gitster@pobox.com>
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     - ## Documentation/config/sideband.txt ##
     -@@ Documentation/config/sideband.txt: sideband.allowControlCharacters::
     + ## Documentation/config/sideband.adoc ##
     +@@ Documentation/config/sideband.adoc: sideband.allowControlCharacters::
       	`true`::
       		Allow all control characters to be sent to the terminal.
       --
     @@ sideband.c: static void parse_allow_control_characters(const char *value)
      +	config.collect_fn = sideband_config_callback;
      +
      +	normalized_url = url_normalize(url, &config.url);
     -+	git_config(urlmatch_config_entry, &config);
     ++	repo_config(the_repository, urlmatch_config_entry, &config);
      +	free(normalized_url);
      +	string_list_clear(&config.vars, 1);
      +	urlmatch_config_release(&config);
       }
       
       /* Returns a color setting (GIT_COLOR_NEVER, etc). */
     -@@ sideband.c: static int use_sideband_colors(void)
     - 	if (use_sideband_colors_cached >= 0)
     +@@ sideband.c: static enum git_colorbool use_sideband_colors(void)
     + 	if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
       		return use_sideband_colors_cached;
       
     --	switch (git_config_get_maybe_bool("sideband.allowcontrolcharacters", &i)) {
     +-	switch (repo_config_get_maybe_bool(the_repository, "sideband.allowcontrolcharacters", &i)) {
      -	case 0: /* Boolean value */
      -		allow_control_characters = i ? ALLOW_ALL_CONTROL_CHARACTERS :
      -			ALLOW_NO_CONTROL_CHARACTERS;
      -		break;
      -	case -1: /* non-Boolean value */
     --		if (git_config_get_string_tmp("sideband.allowcontrolcharacters",
     +-		if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters",
      -					      &value))
      -			; /* huh? `get_maybe_bool()` returned -1 */
      -		else
     @@ sideband.c: static int use_sideband_colors(void)
      -	default:
      -		break; /* not configured */
      +	if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) {
     -+		if (!git_config_get_value("sideband.allowcontrolcharacters", &value))
     ++		if (!repo_config_get_value(the_repository, "sideband.allowcontrolcharacters", &value))
      +			sideband_allow_control_characters_config("sideband.allowcontrolcharacters", value);
      +
      +		if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET)
      +			allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
       	}
       
     - 	if (!git_config_get_string_tmp(key, &value))
     + 	if (!repo_config_get_string_tmp(the_repository, key, &value))
      
       ## sideband.h ##
      @@ sideband.h: int demultiplex_sideband(const char *me, int status,
     @@ transport.c
       #include "bundle-uri.h"
      +#include "sideband.h"
       
     - static int transport_use_color = -1;
     + static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
       static char transport_colors[][COLOR_MAXLEN] = {
      @@ transport.c: struct transport *transport_get(struct remote *remote, const char *url)
       
     - 	ret->hash_algo = &hash_algos[GIT_HASH_SHA1];
     + 	ret->hash_algo = &hash_algos[GIT_HASH_SHA1_LEGACY];
       
      +	sideband_apply_url_config(ret->url);
      +
 -:  ---------- > 6:  8b8244eca9 sideband: delay sanitizing by default to Git v3.0

-- 
gitgitgadget