Thread (47 messages) flat view 47 messages, 7 authors, 2026-06-11
STALE93d

Revision v5 of 2 in this series.

Revisions (2)
  1. v1 [diff vs current]
  2. v5 current

[PATCH v5 0/7] Sanitizing sideband output

From: Junio C Hamano <hidden>
Date: 2026-03-05 23:34:55

Git's sideband channel passes server output directly to the client
terminal without sanitizing it, creating an ANSI escape sequence
injection vulnerability.  A patchset was posted based on the "we
dictate what byte sequences are allowed to be used by the end user,
and reject everything else, but the users can choose to lift the
blanket rejection" approach, found in [*1*], but the discussion
stalled.

Here is an attempt to unblock the topic.  In this round, the series
keeps the "we know what users are allowed to use and reject the
rest" approach from the original, but fixes one design problem
around the configuration variable.  The value 'default' that can be
explicitly set to the configuration whose meaning is planned to
change across Git 3.0 boundary is now gone.

The series is structured as follows.

 * The first 5 patches are from Dscho's [v4 1-5/6].  These introduce
   "reject what we do not explicitly allow" framework, and by
   default allow only ANSI color escape sequences.

   [1/7] sideband: mask control characters
   [2/7] sideband: introduce an "escape hatch" to allow control characters
   [3/7] sideband: do allow ANSI color sequences by default
   [4/7] sideband: add options to allow more control sequences to be passed through
   [5/7] sideband: offer to configure sanitizing on a per-URL basis

 * The 'default' problem was raised in [*2*] and the 6th patch is
   about solving it.

   [6/7] sideband: drop 'default' configuration

 * The 7th patch is from Dscho's [v4 6/6], adjusted for the above
   step.  This loosens the rules to allow _everything_ (i.e., the
   same as before) before Git 3.0, and then rejects anything other
   than ANSI color escape sequences after Git 3.0

   [7/7] sideband: delay sanitizing by default to Git v3.0

I think without the 'default' that changes its meaning across
version boundary, the early part of the series is a good place to
stop, if we want to proceed with the "allowlist" approach.  Unless I
hear objections, let's plan to merge patches 1-6 (i.e., everything
other than ANSI color escape sequences are filtered out) to 'next'
and keep it there for some time, to see if anybody screams, without
the last patch.

I am still hoping that people can help the topic and make it less
risky for users who are affected by switching to an opposite
philosophy, namely, "we filter byte sequences that are known to us
to be problematic, and pass everything else intact", but that is a
separate topic [*2*].  The experiment with patches 1-6 cooked in
'next' long enough may convince ourselves that ANSI colors are the
only thing that needs passing (which I personally fear is a bit too
naïve but we will never know until we try).


[References]

 *1* https://lore.kernel.org/git/pull.1853.v4.git.1770113882.gitgitgadget@gmail.com/ (local)

 *2* https://lore.kernel.org/git/xmqqv7gcnwd4.fsf@gitster.g/ (local)

-- 
2.53.0-629-gb58d2f6a3e
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help