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

[Leo Prikler]

Am Donnerstag, den 22.04.2021, 22:01 +0200 schrieb Léo Le Bouter:
> On Thu, 2021-04-22 at 00:08 -0400, Mark H Weaver wrote:
> > 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
> 
> Hello Mark,
> 
> I don't share your analysis, the security fixes werent stripped
> because
> glib/cairo was also updated to latest version in subsequent commits
> which were pushed all at once.
This may be the case for glib, but which commit pushes cairo to a
version, in which those security fixes are applied?