Re: A "cosmetic changes" commit that removes security fixes

[Mark H Weaver]

Hi Raghav,

Raghav Gururajan <rg@raghavgururajan.name> writes:

>> Those commits on 'core-updates' were digitally signed by Léo Le Bouter
>> <lle-bout@zaclys.net> and have the same problems: they remove security
>> fixes, and yet the summary lines indicate that only "cosmetic changes"
>> were made.
>
> Yeah, the commit title didn't mention the change but the commit message did.

I'm sorry, but that won't do.  There are at least three things wrong
with these commits:

(1) The summary lines were misleading, because they implied that no
    functional changes were made.

(2) The commit messages were misleading, because they failed to mention
    that security holes which had previously been fixed were now being
    re-introduced.  That wasn't at all obvious.

    Commits like these, which remove patches that had fixed security
    flaws, are fairly common: someone casually looking over the commit
    log might assume that the patches could be safely removed because a
    version update was done at the same time, rendering those patches
    obsolete.

(3) Although your 'glib' commit was immediately followed by a 'glib'
    update, rendering it harmless, your misleading 'cairo' commit left
    'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our
    'core-updates' and 'wip-gnome' branches.  Those will need to be
    fixed now.

Léo Le Bouter <lle-bout@zaclys.net> is also culpable here, because he
digitally signed the misleading 'cairo' commit that's on our
'core-updates' branch, which re-introduced CVE-2018-19876 and
CVE-2020-35492.

--8<---------------cut here---------------start------------->8---
commit f94cdc86f644984ca83164d40b17e7eed6e22091
gpg: Signature made Fri 26 Mar 2021 05:13:57 PM EDT
gpg:                using RSA key 148BCB8BD80BFB16B1DE0E9145A8B1E86BCD10A6
gpg: Good signature from "Léo Le Bouter <lle-bout@zaclys.net>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 148B CB8B D80B FB16 B1DE  0E91 45A8 B1E8 6BCD 10A6
Author: Raghav Gururajan <raghavgururajan@disroot.org>
Date:   Fri Dec 4 00:48:43 2020 -0500

    gnu: cairo: Make some cosmetic changes.
    
    * gnu/packages/patches/cairo-CVE-2018-19876.patch,
    gnu/packages/patches/cairo-CVE-2020-35492.patch: Remove patches.
    * gnu/local.mk (dist_patch_DATA): Unregister them.
    * gnu/packages/gtk.scm (cairo): Make some cosmetic changes.
    [replacement]: Remove.
    (cairo/fixed): Remove.
    
    Signed-off-by: Léo Le Bouter <lle-bout@zaclys.net>
--8<---------------cut here---------------end--------------->8---

https://git.sv.gnu.org/cgit/guix.git/commit/?h=core-updates&id=f94cdc86f644984ca83164d40b17e7eed6e22091

Even the most superficial skimming of this commit should have
immediately raised red flags, because the summary line is clearly
inaccurate.  It shows a lack of careful review, to put it mildly.

      Mark