sideband: add options to allow more control sequences to be passed through

Even though control sequences that erase characters are quite juicy for
attack scenarios, where attackers are eager to hide traces of suspicious
activities, during the review of the side band sanitizing patch series
concerns were raised that there might be some legimitate scenarios where
Git server's `pre-receive` hooks use those sequences in a benign way.

Control sequences to move the cursor can likewise be used to hide tracks
by overwriting characters, and have been equally pointed out as having
legitimate users.

Let's add options to let users opt into passing through those ANSI
Escape sequences: `sideband.allowControlCharacters` now supports also
`cursor` and `erase`, and it parses the value as a comma-separated list.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index b55c73726fe2c76a4188a289a0e11aad844477af..2bf04262840b020e9a46283eb0a0796527bb8a6c 100644 (file)
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -2,13 +2,20 @@ 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
-       this config setting to override this behavior:
+       this config setting to override this behavior (the value can be
+       a comma-separated list of the following keywords):
 +
 --
        `default`::
        `color`::
                Allow ANSI color sequences, line feeds and horizontal tabs,
                but mask all other control characters. This is the default.
+       `cursor:`:
+               Allow control sequences that move the cursor. This is
+               disabled by default.
+       `erase`::
+               Allow control sequences that erase charactrs. This is
+               disabled by default.
        `false`::
                Mask all control characters other than line feeds and
                horizontal tabs.

diff --git a/sideband.c b/sideband.c
index eeba6fa2ca8dd6f306b8d1d5eff4ce3601ff0011..0b420ca3193888f4e1acd5d8f4f0a2bc887243ca 100644 (file)
--- a/sideband.c
+++ b/sideband.c
@@ -29,9 +29,43 @@ static struct keyword_entry keywords[] = {
 static enum {
        ALLOW_NO_CONTROL_CHARACTERS  = 0,
        ALLOW_ANSI_COLOR_SEQUENCES   = 1<<0,
+       ALLOW_ANSI_CURSOR_MOVEMENTS  = 1<<1,
+       ALLOW_ANSI_ERASE             = 1<<2,
        ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
-       ALLOW_ALL_CONTROL_CHARACTERS = 1<<1,
-} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
+       ALLOW_ALL_CONTROL_CHARACTERS = 1<<3,
+} allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
+
+static inline int skip_prefix_in_csv(const char *value, const char *prefix,
+                                    const char **out)
+{
+       if (!skip_prefix(value, prefix, &value) ||
+           (*value && *value != ','))
+               return 0;
+       *out = value + !!*value;
+       return 1;
+}
+
+static void parse_allow_control_characters(const char *value)
+{
+       allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
+       while (*value) {
+               if (skip_prefix_in_csv(value, "default", &value))
+                       allow_control_characters |= ALLOW_DEFAULT_ANSI_SEQUENCES;
+               else if (skip_prefix_in_csv(value, "color", &value))
+                       allow_control_characters |= ALLOW_ANSI_COLOR_SEQUENCES;
+               else if (skip_prefix_in_csv(value, "cursor", &value))
+                       allow_control_characters |= ALLOW_ANSI_CURSOR_MOVEMENTS;
+               else if (skip_prefix_in_csv(value, "erase", &value))
+                       allow_control_characters |= ALLOW_ANSI_ERASE;
+               else if (skip_prefix_in_csv(value, "true", &value))
+                       allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
+               else if (skip_prefix_in_csv(value, "false", &value))
+                       allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
+               else
+                       warning(_("unrecognized value for `sideband."
+                                 "allowControlCharacters`: '%s'"), value);
+       }
+}
 
 /* Returns a color setting (GIT_COLOR_NEVER, etc). */
 static enum git_colorbool use_sideband_colors(void)
@@ -55,13 +89,8 @@ 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"))
-                       allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
-               else if (!strcmp(value, "color"))
-                       allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
                else
-                       warning(_("unrecognized value for `sideband."
-                                 "allowControlCharacters`: '%s'"), value);
+                       parse_allow_control_characters(value);
                break;
        default:
                break; /* not configured */
@@ -94,7 +123,7 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
                list_config_item(list, prefix, keywords[i].keyword);
 }
 
-static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int n)
+static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)
 {
        int i;
 
@@ -106,14 +135,47 @@ static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int
         * These are part of the Select Graphic Rendition sequences which
         * contain more than just color sequences, for more details see
         * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.
+        *
+        * The cursor movement sequences are:
+        *
+        * ESC [ n A - Cursor up n lines (CUU)
+        * ESC [ n B - Cursor down n lines (CUD)
+        * ESC [ n C - Cursor forward n columns (CUF)
+        * ESC [ n D - Cursor back n columns (CUB)
+        * ESC [ n E - Cursor next line, beginning (CNL)
+        * ESC [ n F - Cursor previous line, beginning (CPL)
+        * ESC [ n G - Cursor to column n (CHA)
+        * ESC [ n ; m H - Cursor position (row n, col m) (CUP)
+        * ESC [ n ; m f - Same as H (HVP)
+        *
+        * The sequences to erase characters are:
+        *
+        *
+        * ESC [ 0 J - Clear from cursor to end of screen (ED)
+        * ESC [ 1 J - Clear from cursor to beginning of screen (ED)
+        * ESC [ 2 J - Clear entire screen (ED)
+        * ESC [ 3 J - Clear entire screen + scrollback (ED) - xterm extension
+        * ESC [ 0 K - Clear from cursor to end of line (EL)
+        * ESC [ 1 K - Clear from cursor to beginning of line (EL)
+        * ESC [ 2 K - Clear entire line (EL)
+        * ESC [ n M - Delete n lines (DL)
+        * ESC [ n P - Delete n characters (DCH)
+        * ESC [ n X - Erase n characters (ECH)
+        *
+        * For a comprehensive list of common ANSI Escape sequences, see
+        * https://www.xfree86.org/current/ctlseqs.html
         */
 
-       if (allow_control_characters != ALLOW_ANSI_COLOR_SEQUENCES ||
-           n < 3 || src[0] != '\x1b' || src[1] != '[')
+       if (n < 3 || src[0] != '\x1b' || src[1] != '[')
                return 0;
 
        for (i = 2; i < n; i++) {
-               if (src[i] == 'm') {
+               if (((allow_control_characters & ALLOW_ANSI_COLOR_SEQUENCES) &&
+                    src[i] == 'm') ||
+                   ((allow_control_characters & ALLOW_ANSI_CURSOR_MOVEMENTS) &&
+                    strchr("ABCDEFGHf", src[i])) ||
+                   ((allow_control_characters & ALLOW_ANSI_ERASE) &&
+                    strchr("JKMPX", src[i]))) {
                        strbuf_add(dest, src, i + 1);
                        return i;
                }
@@ -128,7 +190,7 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
 {
        int i;
 
-       if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) {
+       if ((allow_control_characters & ALLOW_ALL_CONTROL_CHARACTERS)) {
                strbuf_add(dest, src, n);
                return;
        }
@@ -137,7 +199,8 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
        for (; n && *src; src++, n--) {
                if (!iscntrl(*src) || *src == '\t' || *src == '\n') {
                        strbuf_addch(dest, *src);
-               } else if ((i = handle_ansi_color_sequence(dest, src, n))) {
+               } else if (allow_control_characters != ALLOW_NO_CONTROL_CHARACTERS &&
+                          (i = handle_ansi_sequence(dest, src, n))) {
                        src += i;
                        n -= i;
                } else {

diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index e5092d3b426cd3c7f54cce13081110ed19223a6c..896e790bf955cd1bccf1a253349ac826a23df4b2 100755 (executable)
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -128,4 +128,42 @@ test_expect_success 'disallow (color) control sequences in sideband' '
        test_file_not_empty actual
 '
 
+test_decode_csi() {
+       awk '{
+               while (match($0, /\033/) != 0) {
+                       printf "%sCSI ", substr($0, 1, RSTART-1);
+                       $0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
+               }
+               print
+       }'
+}
+
+test_expect_success 'control sequences in sideband allowed by default' '
+       write_script .git/color-me-surprised <<-\EOF &&
+       printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
+       exec "$@"
+       EOF
+       test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
+       test_commit need-at-least-one-commit-at-least &&
+
+       rm -rf throw-away &&
+       git clone --no-local . throw-away 2>stderr &&
+       test_decode_color <stderr >color-decoded &&
+       test_decode_csi <color-decoded >decoded &&
+       test_grep ! "CSI \\[K" decoded &&
+       test_grep ! "CSI \\[G" decoded &&
+       test_grep "\\^\\[?25l" decoded &&
+
+       rm -rf throw-away &&
+       git -c sideband.allowControlCharacters=erase,cursor,color \
+               clone --no-local . throw-away 2>stderr &&
+       test_decode_color <stderr >color-decoded &&
+       test_decode_csi <color-decoded >decoded &&
+       test_grep "RED" decoded &&
+       test_grep "CSI \\[K" decoded &&
+       test_grep "CSI \\[G" decoded &&
+       test_grep ! "\\^\\[\\[K" decoded &&
+       test_grep ! "\\^\\[\\[G" decoded
+'
+
 test_done