Bug#1078883: diffoscope: FTBFS: failing tests

[Fay Stegerman]

* Fay Stegerman <flx at obfusk.net> [2024-08-19 15:40]:
> * Chris Lamb <lamby at debian.org> [2024-08-19 12:37]:
> > Santiago Vila wrote:
> >
> > > E zipfile.BadZipFile: Overlapped entries: 'dir/text' (possible zip bomb)
> >
> > (FAO to help folks joining the thread: this was from a rebuild of
> > stable, not of unstable. This bug does not affect unstable nor
> > testing.)
> >
> > So, this is essentially the same issue as #1068705, which we believe
> > was caused by a regression in CPython [0] which was, in turn, caused
> > by an attempt to make the handling of .zip file safer [1].
> >
> > We worked around this in diffoscope by catching the exception [2].
> > Then, we added a visible user note that we had done so [3].
> >
> > I think we have four options:
> >
> >   1. Revert the security-related changes in CPython.
> >   2. Write and apply a patch to CPython to fix the CPython regression.
> >   3. Backport the two patches (or just the second [2]) to stable.
> >   4. Do nothing and accept that diffoscope FTBFS in stable.
> >
> > (1) is pretty much a no-go, and then I don't think a patch to (2) will
> > be forthcoming as I lack the confidence to safely write one. And (4)
> > only works if we think that someone will effect (2) for us, will be
> > backported by the CPython devs _and_ it will land in bookworm soon. A
> > tall order.
> >
> > (3) is thus probably the best plan. The first (or both) of the
> > linked changes [2][3] could straightforwardly and safely be backported
> > to stable… if folks that it is justified. Let me know.
> >
> >  [0] https://github.com/python/cpython/issues/117779
> >  [1] https://github.com/python/cpython/pull/110016
> >  [2] https://salsa.debian.org/reproducible-builds/diffoscope/commit/9c7e817c79f19e67e56d564b55b728a54a35423b
> >  [3] https://salsa.debian.org/reproducible-builds/diffoscope/-/merge_requests/140/diffs
> 
> Actually, whilst the cause is also cpython#110016, this is [1], not #1068705.
> This one is not a regression (which presumably means (1) and (2) do not apply)
> as it only affects our MozillaZipContainer monkeypatch, which is not exactly
> supported by upstream cpython as these are not valid ZIP files.  So the relevant
> patch to backport is [2].
> 
> Semi-related, I'm wondering if the removal of marshal.load() [3] is worth
> backporting at the same time, given the security implications.
> 
> - Fay
> 
> [1] https://salsa.debian.org/reproducible-builds/diffoscope/-/issues/362
> [2] https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/cc3b077f6ef97b4e20036e9823926fe633c7d4d0
> [3] https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/e75871b07e09cfd778181d905f540a15bd71e63a

For completeness: had this been #1068705 (or in case someone feels it worth to
fix that one too since it is the result of the same Python change even though
it's not related to the FTBFS), the most relevant patch would have been [1]
(though catching the exception would at least prevent a crash).

- Fay

[1] https://salsa.debian.org/reproducible-builds/diffoscope/-/commit/945fd9faed3ae48d5f16602db1eb037c4785aeb5