RPMs/git
Watch
1
0
Fork
You've already forked git
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
git/git-2.49-sanitize-sideband-channel-messages.patch
Ondřej Pohořelský a0b3d1a02b 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.
2025-03-24 12:17:11 +01:00

471 lines
16 KiB
Diff
Raw Blame History

From 328ff864183cdd0a4b779b5b88a3271b39a1b1a2 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 6 Nov 2024 20:34:50 +0100
Subject: [PATCH 1/4] sideband: mask control characters
The output of `git clone` is a vital component for understanding what
has happened when things go wrong. However, these logs are partially
under the control of the remote server (via the "sideband", which
typically contains what the remote `git pack-objects` process sends to
`stderr`), and is currently not sanitized by Git.
This makes Git susceptible to ANSI escape sequence injection (see
CWE-150, https://cwe.mitre.org/data/definitions/150.html), which allows
attackers to corrupt terminal state, to hide information, and even to
insert characters into the input buffer (i.e. as if the user had typed
those characters).
To plug this vulnerability, disallow any control character in the
sideband, replacing them instead with the common `^<letter/symbol>`
(e.g. `^[` for `\x1b`, `^A` for `\x01`).
There is likely a need for more fine-grained controls instead of using a
"heavy hammer" like this, which will be introduced subsequently.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sideband.c | 17 +++++++++++++++--
t/t5409-colorize-remote-messages.sh | 12 ++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/sideband.c b/sideband.c
index 251e9615ed..81b1ff0805 100644
--- a/sideband.c
+++ b/sideband.c
@@ -66,6 +66,19 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
list_config_item(list, prefix, keywords[i].keyword);
}
+static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
+{
+ strbuf_grow(dest, n);
+ for (; n && *src; src++, n--) {
+ if (!iscntrl(*src) || *src == '\t' || *src == '\n')
+ strbuf_addch(dest, *src);
+ 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
@@ -81,7 +94,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;
}
@@ -114,7 +127,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..d0745c391b 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -98,4 +98,16 @@ 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?\\n" >&2
+ exec "$@"
+ EOF
+ test_config_global uploadPack.packObjectshook ./color-me-surprised &&
+ test_commit need-at-least-one-commit &&
+ git clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >decoded &&
+ test_grep ! RED decoded
+'
+
test_done
--
2.49.0
From ab2eb6c0043c643935ea0fbdaed68e15bc831b11 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 6 Nov 2024 21:07:51 +0100
Subject: [PATCH 2/4] sideband: introduce an "escape hatch" to allow control
characters
The preceding commit fixed the vulnerability whereas sideband messages
(that are under the control of the remote server) could contain ANSI
escape sequences that would be sent to the terminal verbatim.
However, this fix may not be desirable under all circumstances, e.g.
when remote servers deliberately add coloring to their messages to
increase their urgency.
To help with those use cases, give users a way to opt-out of the
protections: `sideband.allowControlCharacters`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/config.adoc | 2 ++
Documentation/config/sideband.adoc | 5 +++++
sideband.c | 10 ++++++++++
t/t5409-colorize-remote-messages.sh | 8 +++++++-
4 files changed, 24 insertions(+), 1 deletion(-)
create mode 100644 Documentation/config/sideband.adoc
diff --git a/Documentation/config.adoc b/Documentation/config.adoc
index cc769251be..a8b04c4e51 100644
--- a/Documentation/config.adoc
+++ b/Documentation/config.adoc
@@ -522,6 +522,8 @@ include::config/sequencer.adoc[]
include::config/showbranch.adoc[]
+include::config/sideband.adoc[]
+
include::config/sparse.adoc[]
include::config/splitindex.adoc[]
diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
new file mode 100644
index 0000000000..3fb5045cd7
--- /dev/null
+++ b/Documentation/config/sideband.adoc
@@ -0,0 +1,5 @@
+sideband.allowControlCharacters::
+ By default, control characters that are delivered via the sideband
+ are masked, to prevent potentially unwanted ANSI escape sequences
+ from being sent to the terminal. Use this config setting to override
+ this behavior.
diff --git a/sideband.c b/sideband.c
index 81b1ff0805..d1c326fa19 100644
--- a/sideband.c
+++ b/sideband.c
@@ -26,6 +26,8 @@ static struct keyword_entry keywords[] = {
{ "error", GIT_COLOR_BOLD_RED },
};
+static int allow_control_characters;
+
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
static int use_sideband_colors(void)
{
@@ -39,6 +41,9 @@ static int use_sideband_colors(void)
if (use_sideband_colors_cached >= 0)
return use_sideband_colors_cached;
+ git_config_get_bool("sideband.allowcontrolcharacters",
+ &allow_control_characters);
+
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))
@@ -68,6 +73,11 @@ 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)
{
+ if (allow_control_characters) {
+ strbuf_add(dest, src, n);
+ return;
+ }
+
strbuf_grow(dest, n);
for (; n && *src; src++, n--) {
if (!iscntrl(*src) || *src == '\t' || *src == '\n')
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index d0745c391b..fb31e85254 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -105,9 +105,15 @@ test_expect_success 'disallow (color) control sequences in sideband' '
EOF
test_config_global uploadPack.packObjectshook ./color-me-surprised &&
test_commit need-at-least-one-commit &&
+
git clone --no-local . throw-away 2>stderr &&
test_decode_color <stderr >decoded &&
- test_grep ! RED decoded
+ test_grep ! RED decoded &&
+
+ rm -rf throw-away &&
+ git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr &&
+ test_decode_color <stderr >decoded &&
+ test_grep RED decoded
'
test_done
--
2.49.0
From a369672c2e6974590ad0561854318a4f255e6893 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 18 Nov 2024 21:42:57 +0100
Subject: [PATCH 3/4] sideband: do allow ANSI color sequences by default
The preceding two commits introduced special handling of the sideband
channel to neutralize ANSI escape sequences before sending the payload
to the terminal, and `sideband.allowControlCharacters` to override that
behavior.
However, some `pre-receive` hooks that are actively used in practice
want to color their messages and therefore rely on the fact that Git
passes them through to the terminal.
In contrast to other ANSI escape sequences, it is highly unlikely that
coloring sequences can be essential tools in attack vectors that mislead
Git users e.g. by hiding crucial information.
Therefore we can have both: Continue to allow ANSI coloring sequences to
be passed to the terminal, and neutralize all other ANSI escape
sequences.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/config/sideband.adoc | 17 ++++++--
sideband.c | 61 ++++++++++++++++++++++++++---
t/t5409-colorize-remote-messages.sh | 16 +++++++-
3 files changed, 84 insertions(+), 10 deletions(-)
diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index 3fb5045cd7..f347fd6b33 100644
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -1,5 +1,16 @@
sideband.allowControlCharacters::
By default, control characters that are delivered via the sideband
- are masked, to prevent potentially unwanted ANSI escape sequences
- from being sent to the terminal. Use this config setting to override
- this behavior.
+ 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:
++
+--
+ color::
+ Allow ANSI color sequences, line feeds and horizontal tabs,
+ but mask all other control characters. This is the default.
+ false::
+ Mask all control characters other than line feeds and
+ horizontal tabs.
+ true::
+ Allow all control characters to be sent to the terminal.
+--
diff --git a/sideband.c b/sideband.c
index d1c326fa19..9084ca234d 100644
--- a/sideband.c
+++ b/sideband.c
@@ -26,7 +26,11 @@ static struct keyword_entry keywords[] = {
{ "error", GIT_COLOR_BOLD_RED },
};
-static int allow_control_characters;
+static enum {
+ ALLOW_NO_CONTROL_CHARACTERS = 0,
+ ALLOW_ALL_CONTROL_CHARACTERS = 1,
+ ALLOW_ANSI_COLOR_SEQUENCES = 2
+} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
static int use_sideband_colors(void)
@@ -41,8 +45,24 @@ static int use_sideband_colors(void)
if (use_sideband_colors_cached >= 0)
return use_sideband_colors_cached;
- git_config_get_bool("sideband.allowcontrolcharacters",
- &allow_control_characters);
+ 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);
@@ -71,9 +91,37 @@ 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)
{
- if (allow_control_characters) {
+ int i;
+
+ if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) {
strbuf_add(dest, src, n);
return;
}
@@ -82,7 +130,10 @@ 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 {
+ else if ((i = handle_ansi_color_sequence(dest, src, n))) {
+ src += i;
+ n -= i;
+ } else {
strbuf_addch(dest, '^');
strbuf_addch(dest, 0x40 + *src);
}
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index fb31e85254..a755c49a74 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -100,7 +100,7 @@ test_expect_success 'fallback to color.ui' '
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?\\n" >&2
+ printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
exec "$@"
EOF
test_config_global uploadPack.packObjectshook ./color-me-surprised &&
@@ -108,12 +108,24 @@ test_expect_success 'disallow (color) control sequences in sideband' '
git 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
+ test_grep RED decoded &&
+ tr -dc "\\007" <stderr >actual &&
+ test_file_not_empty actual
'
test_done
--
2.49.0
From b15d2255ed98eb6f75608c2f99f4ea3284ad250e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= <opohorel@redhat.com>
Date: Mon, 24 Mar 2025 10:51:39 +0100
Subject: [PATCH 4/4] sideband: default to allowControlCharacters=true
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We don't want to change the default Git behaviour, just add the option
to filter control characters.
Signed-off-by: Ondřej Pohořelský <opohorel@redhat.com>
---
Documentation/config/sideband.adoc | 8 ++++----
sideband.c | 2 +-
t/t5409-colorize-remote-messages.sh | 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index f347fd6b33..a809e2de89 100644
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -1,16 +1,16 @@
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:
+ 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. This is the default.
+ 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.
--
diff --git a/sideband.c b/sideband.c
index 9084ca234d..456cd3d8bc 100644
--- a/sideband.c
+++ b/sideband.c
@@ -30,7 +30,7 @@ static enum {
ALLOW_NO_CONTROL_CHARACTERS = 0,
ALLOW_ALL_CONTROL_CHARACTERS = 1,
ALLOW_ANSI_COLOR_SEQUENCES = 2
-} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES;
+} allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
static int use_sideband_colors(void)
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index a755c49a74..2d40d8c640 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -106,7 +106,8 @@ test_expect_success 'disallow (color) control sequences in sideband' '
test_config_global uploadPack.packObjectshook ./color-me-surprised &&
test_commit need-at-least-one-commit &&
- git clone --no-local . throw-away 2>stderr &&
+ 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 &&
--
2.49.0
Reference in a new issue View git blame Copy permalink