Re: [PATCH v4 00/16] Start compiling with `-Wsign-compare`

[karthik nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

I tested the changes with clang and GCC and everything seems good. I
went through the changes themselves and have nothing to add.

Thanks

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]