Re: [PATCH v2 4/4] sideband: add options to allow more control sequences to be passed through

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 3, 2026, 07:12 UTC
Message-ID: <22d81c06-6ef8-dadb-5f1c-cd9461bb290d@gmx.de>
In-Reply-To: <xmqqo6m6vdf8.fsf@gitster.g>

Hi Junio,

On Tue, 3 Feb 2026, Junio C Hamano wrote:

Show 13 quoted lines

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > On 2026-01-15 at 21:14:48, Jeff King wrote:
> >> Is there any reason we cannot introduce the new functionality as a
> >> config option but _not_ enable it by default?
> >> 
> >> That gives people the tools to protect themselves if they want to bear
> >> the potential cost. It just feels a shame to deny them the tool because
> >> we can't agree on the default.
> >
> > Yes, I think that would be a fine and reasonable approach.
> 
> Absolutely.

I disagree with neutering this security fix. Let me explain why, and then propose a compromise.

CVE-2024-52005 exists because Git passes untrusted payload to the terminal without sanitization. The terminal interprets control sequences sent to it (it has no way to distinguish between sequences Git _intended_ to send and sequences a malicious remote slipped into the sideband). That responsibility falls squarely on Git. Shifting it to terminal emulators is not viable: they _cannot_ make that distinction.

This is not about one specific vulnerability (OSC 8 or otherwise). It is about the principle that programs must sanitize untrusted input before passing it to an interpreter. Terminal emulators are interpreters. The set of exploitable sequences changes over time as terminals add features; the only durable fix is sanitization at the source.

Now, about breaking existing users.

The patches I submitted _do not_ break pre-receive hooks that emit color sequences. Color sequences are allowed by default. What is disabled by default are sequences that set the window title, query terminal state, move the cursor, etc. (functionality that legitimate hooks have no business using, and functionality that is ripe for exploitation).

The concern about Japanese ISO encodings colliding with control bytes is theoretical at best: sideband messages are prefixed with the ASCII string "remote: ", so any such encoding would already be broken today.

Here is the problem with "off by default".

Turning the sanitization off by default means CVE-2024-52005 remains unaddressed for the vast majority of users. Providing an opt-in config is security theater: users who do not know about the vulnerability will not enable the protection. That is the opposite of defense in depth.

Fedora's decision to ship with sanitization disabled does not validate the approach; it reflects their reluctance to diverge from upstream defaults, not a security analysis concluding that the default is safe.

That said, I can see a path forward.

1. Turn sanitization _off_ by default in 2.x.
2. Document clearly that this default will change.
3. In Git 3.0 (the next breaking-changes release), flip the default so
   that `sideband.allowControlCharacters=color` is the baseline, i.e.
   color sequences pass through, but nothing else does.

This gives users and tooling (and support engineers) time to adapt while committing to secure-by-default behavior in the near future.

I would appreciate hearing from anyone on the list with security expertise. The principle that untrusted input must be sanitized before reaching an interpreter is foundational; I am not aware of any credible security guidance that says otherwise.

Ciao, Dscho