Skip to content

Commit 8b8244e

Browse files
committed
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>
1 parent 692d1a6 commit 8b8244e

File tree

3 files changed

+29
-6
lines changed

3 files changed

+29
-6
lines changed

Documentation/config/sideband.adoc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
sideband.allowControlCharacters::
2+
ifdef::with-breaking-changes[]
23
By default, control characters that are delivered via the sideband
34
are masked, except ANSI color sequences. This prevents potentially
5+
endif::with-breaking-changes[]
6+
ifndef::with-breaking-changes[]
7+
By default, no control characters delivered via the sideband
8+
are masked. This is unsafe and will change in Git v3.* to only
9+
allow ANSI color sequences by default, preventing potentially
10+
endif::with-breaking-changes[]
411
unwanted ANSI escape sequences from being sent to the terminal. Use
512
this config setting to override this behavior (the value can be
613
a comma-separated list of the following keywords):
714
+
815
--
916
`default`::
17+
ifndef::with-breaking-changes[]
18+
Allow any control sequence. This default is unsafe and will
19+
change to `color` in Git v3.*.
20+
endif::with-breaking-changes[]
1021
`color`::
1122
Allow ANSI color sequences, line feeds and horizontal tabs,
1223
but mask all other control characters. This is the default.

sideband.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,12 @@ static enum {
3333
ALLOW_ANSI_COLOR_SEQUENCES = 1<<0,
3434
ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1,
3535
ALLOW_ANSI_ERASE = 1<<2,
36-
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
3736
ALLOW_ALL_CONTROL_CHARACTERS = 1<<3,
37+
#ifdef WITH_BREAKING_CHANGES
38+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
39+
#else
40+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ALL_CONTROL_CHARACTERS,
41+
#endif
3842
} allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
3943

4044
static inline int skip_prefix_in_csv(const char *value, const char *prefix,

t/t5409-colorize-remote-messages.sh

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,13 @@ test_expect_success 'fallback to color.ui' '
9898
grep "<BOLD;RED>error<RESET>: error" decoded
9999
'
100100

101+
if test_have_prereq WITH_BREAKING_CHANGES
102+
then
103+
TURN_ON_SANITIZING=already.turned=on
104+
else
105+
TURN_ON_SANITIZING=sideband.allowControlCharacters=color
106+
fi
107+
101108
test_expect_success 'disallow (color) control sequences in sideband' '
102109
write_script .git/color-me-surprised <<-\EOF &&
103110
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' '
106113
test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
107114
test_commit need-at-least-one-commit &&
108115
109-
git clone --no-local . throw-away 2>stderr &&
116+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
110117
test_decode_color <stderr >decoded &&
111118
test_grep RED decoded &&
112119
test_grep "\\^G" stderr &&
@@ -138,7 +145,7 @@ test_decode_csi() {
138145
}'
139146
}
140147

141-
test_expect_success 'control sequences in sideband allowed by default' '
148+
test_expect_success 'control sequences in sideband allowed by default (in Git v3.8)' '
142149
write_script .git/color-me-surprised <<-\EOF &&
143150
printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
144151
exec "$@"
@@ -147,7 +154,7 @@ test_expect_success 'control sequences in sideband allowed by default' '
147154
test_commit need-at-least-one-commit-at-least &&
148155
149156
rm -rf throw-away &&
150-
git clone --no-local . throw-away 2>stderr &&
157+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
151158
test_decode_color <stderr >color-decoded &&
152159
test_decode_csi <color-decoded >decoded &&
153160
test_grep ! "CSI \\[K" decoded &&
@@ -175,14 +182,15 @@ test_expect_success 'allow all control sequences for a specific URL' '
175182
test_commit one-more-please &&
176183
177184
rm -rf throw-away &&
178-
git clone --no-local . throw-away 2>stderr &&
185+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
179186
test_decode_color <stderr >color-decoded &&
180187
test_decode_csi <color-decoded >decoded &&
181188
test_grep ! "CSI \\[K" decoded &&
182189
test_grep "\\^\\[\\[K" decoded &&
183190
184191
rm -rf throw-away &&
185-
git -c "sideband.file://.allowControlCharacters=true" \
192+
git -c sideband.allowControlCharacters=false \
193+
-c "sideband.file://.allowControlCharacters=true" \
186194
clone --no-local "file://$PWD" throw-away 2>stderr &&
187195
test_decode_color <stderr >color-decoded &&
188196
test_decode_csi <color-decoded >decoded &&

0 commit comments

Comments
 (0)