git/git-2.43.5-sanitize-sideband-channel-messages.patch
Ondřej Pohořelský ade8e35402 add the option to sanitize sideband channel messages
Resolves: RHEL-74177
2025-04-04 11:35:28 +02:00

220 lines
6.7 KiB
Diff

From 13bb730859857c97f298e9a8c7b68fe00074b3d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= <opohorel@redhat.com>
Date: Thu, 3 Apr 2025 14:46:53 +0200
Subject: [PATCH] Adds the option to sanitize sideband channel messages
CVE-2024-52005 wasn't fixed by upstream. This patch adds the option
to harden Git against it.
The default behaviour of Git remains unchanged.
Changes are taken from Git for Windows. The only differences are that
by default we are allowing all control characters, the documentation
reflects it and one of the tests has to be invoked with a config
change: `sideband.allowControlCharacters=color`
These commits can also be seen in this upstream PR:
https://github.com/gitgitgadget/git/pull/1853
---
Documentation/config.txt | 2 +
Documentation/config/sideband.txt | 16 ++++++
sideband.c | 78 ++++++++++++++++++++++++++++-
t/t5409-colorize-remote-messages.sh | 30 +++++++++++
4 files changed, 124 insertions(+), 2 deletions(-)
create mode 100644 Documentation/config/sideband.txt
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e3a74dd1c1..5b8bbdee82 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -513,6 +513,8 @@ include::config/sequencer.txt[]
include::config/showbranch.txt[]
+include::config/sideband.txt[]
+
include::config/sparse.txt[]
include::config/splitindex.txt[]
diff --git a/Documentation/config/sideband.txt b/Documentation/config/sideband.txt
new file mode 100644
index 0000000000..1adc831667
--- /dev/null
+++ b/Documentation/config/sideband.txt
@@ -0,0 +1,16 @@
+sideband.allowControlCharacters::
+ By default, control characters that are delivered via the sideband
+ are NOT masked. Use this config setting to prevent potentially
+ unwanted ANSI escape sequences from being sent to the terminal:
++
+--
+ color::
+ Allow ANSI color sequences, line feeds and horizontal tabs,
+ but mask all other control characters.
+ false::
+ Mask all control characters other than line feeds and
+ horizontal tabs.
+ true::
+ Allow all control characters to be sent to the terminal.
+ This is the default.
+--
\ No newline at end of file
diff --git a/sideband.c b/sideband.c
index 266a67342b..316a401a5d 100644
--- a/sideband.c
+++ b/sideband.c
@@ -23,6 +23,12 @@ static struct keyword_entry keywords[] = {
{ "error", GIT_COLOR_BOLD_RED },
};
+static enum {
+ ALLOW_NO_CONTROL_CHARACTERS = 0,
+ ALLOW_ALL_CONTROL_CHARACTERS = 1,
+ ALLOW_ANSI_COLOR_SEQUENCES = 2
+} allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
+
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
static int use_sideband_colors(void)
{
@@ -36,6 +42,25 @@ static int use_sideband_colors(void)
if (use_sideband_colors_cached >= 0)
return use_sideband_colors_cached;
+ switch (git_config_get_maybe_bool("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",
+ &value))
+ ; /* huh? `get_maybe_bool()` returned -1 */
+ else if (!strcmp(value, "color"))
+ allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
+ else
+ warning(_("unrecognized value for `sideband."
+ "allowControlCharacters`: '%s'"), value);
+ break;
+ default:
+ break; /* not configured */
+ }
+
if (!git_config_get_string(key, &value)) {
use_sideband_colors_cached = git_config_colorbool(key, value);
} else if (!git_config_get_string("color.ui", &value)) {
@@ -64,6 +89,55 @@ 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)
+{
+ int i;
+
+ /*
+ * Valid ANSI color sequences are of the form
+ *
+ * ESC [ [<n> [; <n>]*] m
+ */
+
+ if (allow_control_characters != ALLOW_ANSI_COLOR_SEQUENCES ||
+ n < 3 || src[0] != '\x1b' || src[1] != '[')
+ return 0;
+
+ for (i = 2; i < n; i++) {
+ if (src[i] == 'm') {
+ strbuf_add(dest, src, i + 1);
+ return i;
+ }
+ if (!isdigit(src[i]) && src[i] != ';')
+ break;
+ }
+
+ return 0;
+}
+
+static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
+{
+ int i;
+
+ if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) {
+ strbuf_add(dest, src, n);
+ return;
+ }
+
+ strbuf_grow(dest, 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))) {
+ src += i;
+ n -= i;
+ } else {
+ strbuf_addch(dest, '^');
+ strbuf_addch(dest, 0x40 + *src);
+ }
+ }
+}
+
/*
* Optionally highlight one keyword in remote output if it appears at the start
* of the line. This should be called for a single line only, which is
@@ -79,7 +153,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
int i;
if (!want_color_stderr(use_sideband_colors())) {
- strbuf_add(dest, src, n);
+ strbuf_add_sanitized(dest, src, n);
return;
}
@@ -112,7 +186,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
}
}
- strbuf_add(dest, src, n);
+ strbuf_add_sanitized(dest, src, n);
}
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index fa5de4500a..3b5ff00363 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -98,4 +98,34 @@ test_expect_success 'fallback to color.ui' '
grep "<BOLD;RED>error<RESET>: error" decoded
'
+test_expect_success 'disallow (color) control sequences in sideband' '
+ write_script .git/color-me-surprised <<-\EOF &&
+ printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
+ exec "$@"
+ EOF
+ test_config_global uploadPack.packObjectshook ./color-me-surprised &&
+ test_commit need-at-least-one-commit &&
+ git -c sideband.allowControlCharacters=color \
+ clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >decoded &&
+ test_grep RED decoded &&
+ test_grep "\\^G" stderr &&
+ tr -dc "\\007" <stderr >actual &&
+ test_must_be_empty actual &&
+
+ rm -rf throw-away &&
+ git -c sideband.allowControlCharacters=false \
+ clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >decoded &&
+ test_grep ! RED decoded &&
+ test_grep "\\^G" stderr &&
+
+ rm -rf throw-away &&
+ git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >decoded &&
+ test_grep RED decoded &&
+ tr -dc "\\007" <stderr >actual &&
+ test_file_not_empty actual
+'
+
test_done
--
2.49.0