Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Hi, when compiling with DEVELOPER=YesPlease, we explicitly disable the "-Wsign-compare" warning. This is mostly because our code base is full of cases where we don't bother at all whether something should be signed or unsigned, and enabling the warning would thus cause tons of warnings to pop up. Unfortunately, disabling this warning also masks real issues. There have been multiple CVEs in the Git project that would have been flagged by this warning (e.g. CVE-2022-39260, CVE-2022-41903 and several fixes in the vicinity of these CVEs). Furthermore, the final audit report by X41 D-Sec, who are the ones who have discovered some of the CVEs, hinted that it might be a good idea to become more strict in this context. Now simply enabling the warning globally does not fly due to the stated reason above that we simply have too many sites where we use the wrong integer types. Instead, this patch series introduces a new macro that allows us to explicitly mark files that generate such warnings. Like this, we can adapt the codebase over time and hopefully make this class of vulnerabilities harder to land. Changes in v2: - Explode the 6th patch into multiple patches with more careful analysis and refactorings - Drop the conversion of "gettext.c". To do it properly we'd have to fix the return type of `utf8_strwidth()`, which we have already marked as a todo. - Link to v1: https://lore.kernel.org/r/20241129-pks-sign-compare-v1-0-fc406b984bc9@pks.im Changes in v3: - Include Junio's fix for 32 bit platforms. - Improve a commit message. - Link to v2: https://lore.kernel.org/r/20241202-pks-sign-compare-v2-0-e7f0ad92a749@pks.im Changes in v4: - Remove needless cast in "csum-file.c". - Refactor "pkt-line.c" to be safer regarding integer conversions. - Link to v3: https://lore.kernel.org/r/20241205-pks-sign-compare-v3-0-e211ee089717@pks.im There are a couple of trivial conflicts with kn/midx-wo-the-repository, but I don't think it makes sense to make that a dependency of this series. Let me know in case you disagree and I'll change the base of this series. Thanks! Patrick To: git@vger.kernel.org Cc: shejialuo <shejialuo@gmail.com> Cc: Jeff King <peff@peff.net> Cc: Junio C Hamano <gitster@pobox.com> --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 4, "change-id": "20241128-pks-sign-compare-9cf7412bce1a", "prefixes": [], "history": { "v1": [ "20241129-pks-sign-compare-v1-0-fc406b984bc9@pks.im" ], "v2": [ "20241202-pks-sign-compare-v2-0-e7f0ad92a749@pks.im" ], "v3": [ "20241205-pks-sign-compare-v3-0-e211ee089717@pks.im" ] } } }
- Loading branch information