Re: [PATCH v2 5/8] hook(clone protections): add escape hatch

Hi,

On Sat, 18 May 2024, Junio C Hamano wrote:

To ensure that I don't misunderstand you: You are talking about reverting 8db1e8743c0 (clone: prevent hooks from running during a clone, 2024-03-28), right?

It is _technically_ true that this is not a vulnerability fix. But only _technically_. Practically, it is preventing vulnerabilities from reaching the critical level.

Let's take the most recently-addressed critical vulnerability, CVE-2024-32002. I carefully crafted above-mentioned commit such that it would have prevent the Remote Code Execution attack vector noted in that CVE.

To put this into perspective: If this protection had been put in place before v2.39.4, the CVSS score of CVS-2024-32002 would not have been 9.1 (Critical), but instead 2.2 (Low).

In other words, even if a vulnerability was found, Git's users would have been safer with this defense-in-depth in place.

The same `post-checkout` hook attack vector likewise raised the severity of CVE-2021-21300, CVE-2019-1354, CVE-2019-1353, CVE-2019-1352, and CVE-2019-1349.

It also raised the severity of the vulnerability fixed in v1.8.5.6 (for which we did not obtain a CVE).

Based on past experience, we must expect the semi-steady trickle of Git vulnerabilities to continue, and having this defense-in-depth in place will invaluable in helping to keep the severity of those future vulnerabilities low.

Therefore reverting that commit would put Git's users at risk.

In combination with the fact that we have a path forward via the patches that are under discussion in this here mail thread, a path forward that avoids that risk and incurs an acceptable cost [*1*], the plan to revert 8db1e8743c0 instead should be questioned.

Ciao, Johannes

P.S.: A concern was raised about `safe.hook.sha256` not having tooling to generate those SHA-256 checksums, and putting a burden on 3rd-party tool developers.

However, tooling to generate SHA-256 checksums of files is ubiquitous, there is `sha256sum` and `openssl dgst -sha256`, just to name two tools that are widely available.

And it's not as if _Git users_ would have to generate those checksums. It would be a one-time cost for the developers whose tools install those hooks during a clone (Git offers no option to install hooks during cloning other than templates, which is not broken in v2.39.4, ..., v2.45.1).

Which brings me to that mysterious mention of tools other than Git LFS being potentially affected. It is quite dubitable that tools other than Git LFS use this method of changing the Git repository configuration in such a major way as to install hooks _while running a `smudge` filter_.

It must be put into doubt, too, that this method of abusing the `smudge` filter is the best design to address Git LFS' needs. A serious downside, for example, is that this Git LFS strategy is in conflict with user-provided `post-checkout` hooks that are copied via templates. A better design that comes immediately to mind would be to add a new, generic batched-smudge filter that Git LFS could use, and that would be configured in the system or user-wide config just like Git LFS' `smudge`/`clean` filters, without the need to play hacky games during a clone operation that are very similar to malicious attacks' strategies to abuse Git's hook feature to allow Remote Code Execution.