sideband: delay sanitizing by default to Git v3.0

The sideband sanitization patches allow ANSI color sequences through
by default, preserving compatibility with pre-receive hooks that
provide colored output during `git push`.

Even so, there is concern that changing any default behavior in a
minor release may have unforeseen consequences. To accommodate this,
defer the secure-by-default behavior to Git v3.0, where breaking
changes are expected.

This gives users and tooling time to prepare, while committing to
address CVE-2024-52005 in Git v3.0.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
[jc: adjusted for the removal of 'default' value]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc
index 96fade7f5fee393f5bb78d922925d937380f2f09..ddba93393ccadc0abe1a1406529091409c439661 100644 (file)
--- a/Documentation/config/sideband.adoc
+++ b/Documentation/config/sideband.adoc
@@ -1,8 +1,16 @@
 sideband.allowControlCharacters::
+ifdef::with-breaking-changes[]
        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 (the value can be
+       unwanted ANSI escape sequences from being sent to the terminal.
+endif::with-breaking-changes[]
+ifndef::with-breaking-changes[]
+       By default, no control characters delivered via the sideband
+       are masked. This is unsafe and will change in Git v3.* to only
+       allow ANSI color sequences by default, preventing potentially
+       unwanted ANSI escape sequences from being sent to the terminal.
+endif::with-breaking-changes[]
+       Use this config setting to override this behavior (the value can be
        a comma-separated list of the following keywords):
 +
 --

diff --git a/sideband.c b/sideband.c
index 04282a568edd90de0f46a974435abcffc077c689..5fb60e52bf00b2d110edf32e0f4645096e307cb6 100644 (file)
--- a/sideband.c
+++ b/sideband.c
@@ -34,7 +34,11 @@ static enum {
        ALLOW_ANSI_CURSOR_MOVEMENTS   = 1<<1,
        ALLOW_ANSI_ERASE              = 1<<2,
        ALLOW_ALL_CONTROL_CHARACTERS  = 1<<3,
-       ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ANSI_COLOR_SEQUENCES
+#ifdef WITH_BREAKING_CHANGES
+       ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ANSI_COLOR_SEQUENCES,
+#else
+       ALLOW_DEFAULT_ANSI_SEQUENCES  = ALLOW_ALL_CONTROL_CHARACTERS,
+#endif
 } allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
 
 static inline int skip_prefix_in_csv(const char *value, const char *prefix,

diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index 3010913bb113e4fe37722aa2f1dab6211a3bf11d..07cbc62736bd26ef43037fd1448f409eb8589227 100755 (executable)
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -98,6 +98,13 @@ test_expect_success 'fallback to color.ui' '
        grep "<BOLD;RED>error<RESET>: error" decoded
 '
 
+if test_have_prereq WITH_BREAKING_CHANGES
+then
+       TURN_ON_SANITIZING=already.turned=on
+else
+       TURN_ON_SANITIZING=sideband.allowControlCharacters=color
+fi
+
 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
@@ -106,7 +113,7 @@ 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 $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
        test_decode_color <stderr >decoded &&
        test_grep RED decoded &&
        test_grep "\\^G" stderr &&
@@ -138,7 +145,7 @@ test_decode_csi() {
        }'
 }
 
-test_expect_success 'control sequences in sideband allowed by default' '
+test_expect_success 'control sequences in sideband allowed by default (in Git v3.8)' '
        write_script .git/color-me-surprised <<-\EOF &&
        printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
        exec "$@"
@@ -147,7 +154,7 @@ test_expect_success 'control sequences in sideband allowed by default' '
        test_commit need-at-least-one-commit-at-least &&
 
        rm -rf throw-away &&
-       git clone --no-local . throw-away 2>stderr &&
+       git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
        test_decode_color <stderr >color-decoded &&
        test_decode_csi <color-decoded >decoded &&
        test_grep ! "CSI \\[K" decoded &&
@@ -175,14 +182,15 @@ test_expect_success 'allow all control sequences for a specific URL' '
        test_commit one-more-please &&
 
        rm -rf throw-away &&
-       git clone --no-local . throw-away 2>stderr &&
+       git -c $TURN_ON_SANITIZING 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 "\\^\\[\\[K" decoded &&
 
        rm -rf throw-away &&
-       git -c "sideband.file://.allowControlCharacters=true" \
+       git -c sideband.allowControlCharacters=false \
+               -c "sideband.file://.allowControlCharacters=true" \
                clone --no-local "file://$PWD" throw-away 2>stderr &&
        test_decode_color <stderr >color-decoded &&
        test_decode_csi <color-decoded >decoded &&