Re: Covierty Integration / Improvement

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

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

Hi Markus,

On Fri, 1 Apr 2022, Markus Vervier wrote:

> Dear git developer team,
>
> X41 is processing the current RfP

would you kindly provide a bit more context? This seems to come right out
of left field. Is "RfP" a "Request for Proposals"? If so, I am not aware
that the git developer team submitted one...

> and some questions came up regarding the
> improvement / integration of Coverity Scans for git and the estimation of the
> required work:
>
> - Was there a special purpose for the Coverity integration (e.g. custom
> queries for variant analysis or regression testing?) or did you try to
> integrate it as a best practice / general security hygiene tool?

There has been talk about integrating Coverity into our regular CI builds,
but nothing actionable has materialized yet.

Git for Windows has such a CI build, but it is currently broken due to a
backwards-incompatible change in Coverity that will require a human to
adjust the CI definition, which has not yet happened due to lack of time.

> - Could you tell us more about the amount and types of false positives and
> problems you've faced trying to eliminate them? This will help us to
> understand the expectations / requirements for a successful integration of
> Coverity.

From the top of my head, I would estimate about 60-70% of the results to
be false positives.

As Junio pointed out, we do not consider memory to be leaked in one-shot
processes where memory is allocated, once, in the equivalent of a `main()`
function. Sure, we could add a slew of `free()` calls right before exiting
the process, but that's kind of pointless.

Another major source of false positives is our string data structure,
which offers a small-ish static, read-only buffer to get started, but
replaces that with something `malloc()`ed/`realloc()`ed as soon as the
string is about to be manipulated. Yet Coverity insists that we're writing
into a read-only buffer, and get out of bounds, which is simply not true.

Similar issues are reported with our `strvec` data structure that has the
same allocation pattern.

Since the false positives outnumber the valid issues reported by Coverity,
we have not been very eager to sift through new reports.

The list of categories of false positives listed above is not exhaustive,
of course, but combined with how cumbersome it is to get access to the
reports (they cannot be viewed anonymously), you get an idea why we do not
pay all that much attention to Coverity.

> - Could we get access to a sample of the scan results?

Sure, if you direct your web browser to
https://scan.coverity.com/projects/git, there is a button "Add me to
project".

Ciao,
Johannes

>
> Many Thanks
>
> Markus
> --
> Markus Vervier (Managing Director)
> X41 D-Sec GmbH, Dennewartstr. 25-27, D-52068 Aachen
> T: +49 241 9809418-0, Fax: -9
> Unternehmenssitz: Aachen, Amtsgericht Aachen: HRB19989
> Geschäftsführer: Markus Vervier
>