Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1047370 > unrolled thread

Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087

Back to article view | Back to linux.debian.bugs.dist

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Markus Koschany <apo@debian.org> - 2021-02-10 00:40 +0100
    Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Salvatore Bonaccorso <carnil@debian.org> - 2021-02-10 06:40 +0100
      Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Markus Koschany <apo@debian.org> - 2021-02-10 15:30 +0100
        Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Salvatore Bonaccorso <carnil@debian.org> - 2021-02-10 22:10 +0100
          Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Salvatore Bonaccorso <carnil@debian.org> - 2021-02-11 06:40 +0100
            Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Hugo Lefeuvre <hle@debian.org> - 2021-02-17 08:50 +0100
              Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087 Salvatore Bonaccorso <carnil@debian.org> - 2021-02-25 10:40 +0100

#1047370 — Bug#945317: xcftools NMU for CVE-2019-5086 and CVE-2019-5087

[Multipart message — attachments visible in raw view] — view raw

Control: tags -1 patch pending

Dear maintainer,

I've prepared an NMU for xcftools versioned as 1.0.7-6.1 and
uploaded it to DELAYED/5. Please feel free to tell me if I
should delay it longer.

Regards,

Markus

[toc] | [next] | [standalone]

#1047394

Hi Markus,

On Wed, Feb 10, 2021 at 12:27:38AM +0100, Markus Koschany wrote:
> Control: tags -1 patch pending
> 
> Dear maintainer,
> 
> I've prepared an NMU for xcftools versioned as 1.0.7-6.1 and
> uploaded it to DELAYED/5. Please feel free to tell me if I
> should delay it longer.
> 
> Regards,
> 
> Markus

> diff -Nru xcftools-1.0.7/debian/changelog xcftools-1.0.7/debian/changelog
> --- xcftools-1.0.7/debian/changelog	2016-05-18 12:34:05.000000000 +0200
> +++ xcftools-1.0.7/debian/changelog	2021-02-09 23:15:22.000000000 +0100
> @@ -1,3 +1,16 @@
> +xcftools (1.0.7-6.1) unstable; urgency=high
> +
> +  * Non-maintainer upload by the LTS team.
> +  * Fix CVE-2019-5086 and CVE-2019-5087:
> +    An exploitable integer overflow vulnerability exists in the
> +    flattenIncrementally function in the xcf2png and xcf2pnm binaries of
> +    xcftools. An integer overflow can occur while walking through tiles that
> +    could be exploited to corrupt memory and execute arbitrary code. In order
> +    to trigger this vulnerability, a victim would need to open a specially
> +    crafted XCF file.
> +
> + -- Markus Koschany <apo@debian.org>  Tue, 9 Feb 2021 23:15:22 +0100
> +
>  xcftools (1.0.7-6) unstable; urgency=medium
>  
>    * Team upload (collab-maint)
> diff -Nru xcftools-1.0.7/debian/patches/CVE-2019-5086-and-CVE-2019-5087.patch xcftools-1.0.7/debian/patches/CVE-2019-5086-and-CVE-2019-5087.patch
> --- xcftools-1.0.7/debian/patches/CVE-2019-5086-and-CVE-2019-5087.patch	1970-01-01 01:00:00.000000000 +0100
> +++ xcftools-1.0.7/debian/patches/CVE-2019-5086-and-CVE-2019-5087.patch	2021-02-09 23:15:22.000000000 +0100
> @@ -0,0 +1,60 @@
> +From: Markus Koschany <apo@debian.org>
> +Date: Mon, 8 Feb 2021 17:57:56 +0100
> +Subject: CVE-2019-5086 and CVE-2019-5087
> +
> +Patch by Anton Gladky.
> +
> +Bug-Debian: https://bugs.debian.org/945317
> +Origin: https://github.com/j-jorge/xcftools/pull/15
> +---
> + xcf-general.c | 16 ++++++++++++++++
> + xcftools.h    |  2 +-
> + 2 files changed, 17 insertions(+), 1 deletion(-)
> +
> +diff --git a/xcf-general.c b/xcf-general.c
> +index 9d0b4dc..50be927 100644
> +--- a/xcf-general.c
> ++++ b/xcf-general.c
> +@@ -19,6 +19,7 @@
> + #include "xcftools.h"
> + #include <string.h>
> + #include <errno.h>
> ++#include <limits.h>
> + #ifdef HAVE_ICONV
> + # include <iconv.h>
> + #elif !defined(ICONV_CONST)
> +@@ -182,6 +183,21 @@ xcfString(uint32_t ptr,uint32_t *after)
> + void
> + computeDimensions(struct tileDimensions *d)
> + {
> ++  // [ CVE-2019-5086 and CVE-2019-5087 ]
> ++  // This part of code is the check to prevent integer overflow, see CVE-2019-5086 and CVE-2019-5087
> ++
> ++  if ((d->c.l + d->width)*4 > INT_MAX) {
> ++    fprintf(stderr,("Width is too large (%d)! Stopping execution...\n"), (d->c.l + d->width));
> ++    exit(0);
> ++  }
> ++
> ++  if ((d->c.t + d->height)*4 > INT_MAX) {
> ++    fprintf(stderr,("Height is too large (%d)! Stopping execution...\n"), (d->c.t + d->height));
> ++    exit(0);
> ++  }
> ++
> ++  // [ CVE-2019-5086 and CVE-2019-5087 ]
> ++
> +   d->c.r = d->c.l + d->width ;
> +   d->c.b = d->c.t + d->height ;
> +   d->tilesx = (d->width+TILE_WIDTH-1)/TILE_WIDTH ;
> +diff --git a/xcftools.h b/xcftools.h
> +index e05637a..26d1a23 100644
> +--- a/xcftools.h
> ++++ b/xcftools.h
> +@@ -121,7 +121,7 @@ FILE* openout(const char*);
> + void closeout(FILE *,const char*);
> + 
> + struct rect {
> +-  int t, b, l, r ;
> ++  int64_t t, b, l, r ;
> + };
> + 
> + #define isSubrect(A,B) \
> diff -Nru xcftools-1.0.7/debian/patches/series xcftools-1.0.7/debian/patches/series
> --- xcftools-1.0.7/debian/patches/series	2016-05-18 12:27:32.000000000 +0200
> +++ xcftools-1.0.7/debian/patches/series	2021-02-09 23:15:22.000000000 +0100
> @@ -4,3 +4,4 @@
>  fix-as-needed-linking
>  libpng16.patch
>  fix-test-UTF8.patch
> +CVE-2019-5086-and-CVE-2019-5087.patch

Question back on this.

Is it confirmed that it fixes both CVE-2019-5086 (TALOS-2019-0878,
https://github.com/j-jorge/xcftools/issues/12) and CVE-2019-5087
(TALOS-2019-0879, https://github.com/j-jorge/xcftools/issues/13) which
were slight different issues?

Unfortunately upstream itself is at best dormant.

There was lot of discussion back then basically just around
CVE-2019-5086 but not covering CVE-2019-5087 involving Brian May and a
patch from Anton.

Hugo, Brian, Anton does the final patch you were aiming and posted
address both issues, did any of you got some feedback from Talos
because of TALOS-2019-0878 and TALOS-2019-0879?

Regards,
Salvatore

[toc] | [prev] | [next] | [standalone]

#1047480

[Multipart message — attachments visible in raw view] — view raw

Hello Salvatore,

Am Mittwoch, den 10.02.2021, 06:30 +0100 schrieb Salvatore Bonaccorso:
[...]
> Question back on this.
> 
> Is it confirmed that it fixes both CVE-2019-5086 (TALOS-2019-0878,
> https://github.com/j-jorge/xcftools/issues/12) and CVE-2019-5087
> (TALOS-2019-0879, https://github.com/j-jorge/xcftools/issues/13) which
> were slight different issues?
> 
> Unfortunately upstream itself is at best dormant.
> 
> There was lot of discussion back then basically just around
> CVE-2019-5086 but not covering CVE-2019-5087 involving Brian May and a
> patch from Anton.
> 
> Hugo, Brian, Anton does the final patch you were aiming and posted
> address both issues, did any of you got some feedback from Talos
> because of TALOS-2019-0878 and TALOS-2019-0879?

I was in contact with Anton Gladky and this was basically his patch. The patch
is unfortunately incomplete for 32 bit architectures which I saw too late. I
believe I know how to fix it and I intend to discuss the new patch with Anton.
In my opinion upstream is dead and our Debian maintainer is unresponsive and/or
MIA too, so we have to take the initiative. I haven't contacted TALOS and we
haven't received any feedback from them. The idea behind the patch was to
prevent the overflow with a check for a upper limit for INT. Now, because of
the 32 bit problem, I think we should double-check this with a guard to protect
against negative values for height and width because negative values are
illogical for these dimensions. Currently I believe this fixes both CVE and
Anton had created a test case for it but I check with him again.

Regards,

Markus

[toc] | [prev] | [next] | [standalone]

#1047566

Hi Markus,

On Wed, Feb 10, 2021 at 03:17:57PM +0100, Markus Koschany wrote:
> Hello Salvatore,
> 
> Am Mittwoch, den 10.02.2021, 06:30 +0100 schrieb Salvatore Bonaccorso:
> [...]
> > Question back on this.
> > 
> > Is it confirmed that it fixes both CVE-2019-5086 (TALOS-2019-0878,
> > https://github.com/j-jorge/xcftools/issues/12) and CVE-2019-5087
> > (TALOS-2019-0879, https://github.com/j-jorge/xcftools/issues/13) which
> > were slight different issues?
> > 
> > Unfortunately upstream itself is at best dormant.
> > 
> > There was lot of discussion back then basically just around
> > CVE-2019-5086 but not covering CVE-2019-5087 involving Brian May and a
> > patch from Anton.
> > 
> > Hugo, Brian, Anton does the final patch you were aiming and posted
> > address both issues, did any of you got some feedback from Talos
> > because of TALOS-2019-0878 and TALOS-2019-0879?
> 
> I was in contact with Anton Gladky and this was basically his patch. The patch
> is unfortunately incomplete for 32 bit architectures which I saw too late. I
> believe I know how to fix it and I intend to discuss the new patch with Anton.
> In my opinion upstream is dead and our Debian maintainer is unresponsive and/or
> MIA too, so we have to take the initiative. I haven't contacted TALOS and we
> haven't received any feedback from them. The idea behind the patch was to
> prevent the overflow with a check for a upper limit for INT. Now, because of
> the 32 bit problem, I think we should double-check this with a guard to protect
> against negative values for height and width because negative values are
> illogical for these dimensions. Currently I believe this fixes both CVE and
> Anton had created a test case for it but I check with him again.

Thanks for the status-Update. In short, I cannot claim to judge if the
patch is covering fully both vulnerabilities, but if you are confident
it does then this good. Thing which was unclear is the item Brian
looked at if there are issues with XCF specs.

In a short offlist discussion we (as in security team) discussed in
general about the state of xcftools and we think it might be worth
trying to get it removed from unstable (and so bullseye) in time
because of as said at least dormant, but probably likely dead
upstream.

So checked what would cause problems to have it removed and it looks
not overall too bad. There seem to be only broken build-depends:

castle-game-engine: xcftools
game-data-packager/contrib: xcftools
neurodebian: xcftools

and hopefully all of those can do without it.

castle-game-engine calls in debian/rules xcf2png and maybe this can be
equivalently covered with a imagemagick convert.

For game-data-packager I'm already talking to the right person I guess
:). The Makefile uses here as well xcf2png to convert files.

neurodebian: Again very similar to the others, they use apparently
xcf2png to convert some xcf to png.

But this is something to be detached from the #945317 bugreport, but
it is probably doable and so we can get actually rid of xcftools for
the bullseye release cycle.

I'm not fully in favor to have all the (build-)rdeps forced out of
Debian, that would likely not be a benefit as seems unfair to the
castle-game-engine, game-data-packager and neurodebian packages, but
still think having out xcftools out of bullseye would be the right
thing.

Regards,
Salvatore

[toc] | [prev] | [next] | [standalone]

#1047605

Hi Markus,

On Thu, Feb 11, 2021 at 03:03:19AM +0100, Markus Koschany wrote:
> Hi Salvatore,
> 
> Am Mittwoch, den 10.02.2021, 22:03 +0100 schrieb Salvatore Bonaccorso:
> [...]
> > 
> > I'm not fully in favor to have all the (build-)rdeps forced out of
> > Debian, that would likely not be a benefit as seems unfair to the
> > castle-game-engine, game-data-packager and neurodebian packages, but
> > still think having out xcftools out of bullseye would be the right
> > thing.
> > 
> 
> I believe it makes sense to remove xcftools from Debian because there is a lack
> of upstream support and development but I wouldn't be too aggressive about the
> removal at the moment. My intention is to send a patch to fix the open CVE in
> stable to you when we have addressed the remaining 32 bit issues.

Yes that sounds fine. Admittely it was for us in dsa-needed only
because Hugo initially aimed to adress it across all suites top-down.
It might just be an option to include a fix once it is stable enough
via a point release. But we can look at it once you have a fix as well
for the 32bit issues.

So thanks for working on it!

> I believe it
> would be possible to substitute xcftools with imagemagick but as Simon McVittie
> for game-data-packager already pointed out, the result wouldn't always be the
> same. So a bug report like "Should this package be removed from Debian" with
> severity important should be sufficient for now and bug reports for reverse-
> decencies that highlight the current problems are a good start.

Yes so we have done this, but in double-checking with the release team
the severities are RC (with the clear indication that if it turns not
to be possible, that it can be revisited).

I have seen Simon's reply, so the imagemagick convert variant does not
look the same in in the game-data-packager case actually worse.

It is late in the preparation for bullseye and we should have done
this actually much earlier. So we will see if it is feasible or not
and then have a desirable state or "bite the bullet".

Regards,
Salvatore

[toc] | [prev] | [next] | [standalone]

#1048575

[Multipart message — attachments visible in raw view] — view raw

Hi Salvatore and Markus,

On Thu, Feb 11, 2021 at 06:32:42AM +0100, Salvatore Bonaccorso wrote:
[...]
> On Thu, Feb 11, 2021 at 03:03:19AM +0100, Markus Koschany wrote:
> [...]
> > Am Mittwoch, den 10.02.2021, 22:03 +0100 schrieb Salvatore Bonaccorso:
> > [...]
> > > 
> > > I'm not fully in favor to have all the (build-)rdeps forced out of
> > > Debian, that would likely not be a benefit as seems unfair to the
> > > castle-game-engine, game-data-packager and neurodebian packages, but
> > > still think having out xcftools out of bullseye would be the right
> > > thing.
> > > 
> > 
> > I believe it makes sense to remove xcftools from Debian because there is a lack
> > of upstream support and development but I wouldn't be too aggressive about the
> > removal at the moment. My intention is to send a patch to fix the open CVE in
> > stable to you when we have addressed the remaining 32 bit issues.
> 
> Yes that sounds fine. Admittely it was for us in dsa-needed only
> because Hugo initially aimed to adress it across all suites top-down.
> It might just be an option to include a fix once it is stable enough
> via a point release. But we can look at it once you have a fix as well
> for the 32bit issues.
> 
> So thanks for working on it!

Thanks from my part too! Unfortunately I am struggling to find
time for Debian currently. I makes me feel bad, and I hope that I
will be able to come back soon.

Do you know if xcftools is only used as a build dependency, or is
it used by some end users directly? The popcon is not that low
and my fear is that, even after removing it from Debian, users
would continue to use it, installing from somewhere else,
effectively being at even higher risk than with the Debian
archive's (semi-) patched version.

Of course if we can't offer any support I guess it's still better
to get rid of it than giving a false impression of
support/security.

Best,
Hugo

-- 
                Hugo Lefeuvre (hle)    |    www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C

[toc] | [prev] | [next] | [standalone]

#1049763

Hi Markus,

On Thu, Feb 25, 2021 at 09:11:47AM +0100, Markus Koschany wrote:
> Hello security team, hello Hugo, I hope you are doing well!
> 
> I have just uploaded a NMU for xcftools fixing CVE-2019-5086 and CVE-2019-5087. 
> The new patch also addresses the 32 bit portability issues. The basic idea
> behind it is to limit possible values of width and height (which can only be
> positive) and the offset (which can be positive and negative) to one quarter of
> INT_MAX/INT_MIN. This works for reasonably large images but will of course fail
> for some extreme corner cases now. Since the computeDimensions function is
> responsible for determining these values, you can find the guards there. I have
> attached the POC from Anton Gladky (a manipulated xcf file) which simulates
> extremely large values for height and width. To reproduce the guard is working:
> 
>         xcfinfo small_manipulated.xcf
> 	xcf2png -o test.png small_manipulated.xcf -C
> 
> I propose to use this patch also for Buster either for a point update or
> security release. Feedback always welcome.

Thanks for taking care of it, we think at this point an update in
stable is enough via an upcoming point release, so I would like to
route you there.

Regards,
Salvatore

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web