add the option to sanitize sideband channel messages

Resolves: RHEL-74175
This commit is contained in:
Ondřej Pohořelský 2025-03-31 14:54:36 +02:00
parent a458d1680c
commit 17cf9f8684
2 changed files with 231 additions and 1 deletions

View File

@ -0,0 +1,219 @@
From 833c73801527b37d9bc725c81c6042ae350aaae3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= <opohorel@redhat.com>
Date: Fri, 28 Mar 2025 13:26:29 +0100
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 8c0b3ed807..48870bb588 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -522,6 +522,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 02805573fa..7a0ca61948 100644
--- a/sideband.c
+++ b/sideband.c
@@ -25,6 +25,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)
{
@@ -38,6 +44,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_tmp(key, &value))
use_sideband_colors_cached = git_config_colorbool(key, value);
else if (!git_config_get_string_tmp("color.ui", &value))
@@ -65,6 +90,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
@@ -80,7 +154,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;
}
@@ -113,7 +187,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 516b22fd96..48f8413eff 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -99,4 +99,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

View File

@ -79,7 +79,7 @@
Name: git
Version: 2.47.1
Release: 1%{?dist}
Release: 2%{?dist}
Summary: Fast Version Control System
License: BSD-3-Clause AND GPL-2.0-only AND GPL-2.0-or-later AND LGPL-2.1-or-later AND MIT
URL: https://git-scm.com/
@ -132,6 +132,13 @@ Patch3: 0003-t-lib-git-svn-try-harder-to-find-a-port.patch
# Prevents t5540 failures on i686, s390x and ppc64le
Patch5: git-test-apache-davlockdbtype-config.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.
#
# https://github.com/gitgitgadget/git/pull/1853
Patch6: git-2.47-sanitize-sideband-channel-messages.patch
%if %{with docs}
# pod2man is needed to build Git.3pm
BuildRequires: perl-podlators
@ -1040,6 +1047,10 @@ rmdir --ignore-fail-on-non-empty "$testdir"
%{?with_docs:%{_pkgdocdir}/git-svn.html}
%changelog
* Mon Mar 31 2025 Ondřej Pohořelský <opohorel@redhat.com> - 2.47.1-2
- add the option to sanitize sideband channel messages
- Resolves: RHEL-74175
* Thu Nov 28 2024 Ondřej Pohořelský <opohorel@redhat.com> - 2.47.1-1
- update to 2.47.1
- Resolves: RHEL-63966