[3.6] bpo-36247: zipfile - extract truncates (existing) file when bad password provided (zip encryption weakness)

[CristiFati]

As also specified in the issue, the details are on [SO]: zipfile.BadZipFile: Bad CRC-32 when extracting a password protected .zip & .zip goes corrupt on extract (@CristiFati's answer).

It's about Python 3.6, but it applies to any (actual) version.

This PR, attempts to fix the problem, and also provides a new test (for this specific scenario).
There's a great chance the problem affects a wider variety of scenarios, but tried to limit the fix as possible to this one (to avoid introducing regressions - maybe some other code relies on this behavior, although I doubt it).

Regarding the "xxx" password, I did some tests and this was the 1st one that reproduced it. I tried with generated one char passwords, but I didn't run into the problem. Anyway, it's a coincidence (and I guess, not so important).

Notes:

• There will be a performance decrease (when extracting password protected files), due to additional operations (especially renames - who work with disks). But since files should be on the same FS (partition), there should be only metadata changes
• A regression would be: when the file to be extracted fits on the disk (size-wise), but not together with the existing one. In this case extraction will fail (previously it would have succeeded), but the good thing is that the old one will still be preserver. User deleting the previous file would fix the problem

https://bugs.python.org/issue36247

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

[tirkarthi]

Please open the PR against master branch which I guess is also affected and the fix will be backported to relevant branches once it's accepted by core developer. Reference : https://devguide.python.org/pullrequest/

Thanks

[Arszilla]

Be sure to sign the CLA. Otherwise I don't think they'll accept your fix.

[CristiFati]

@tirkarthi: Shouldn't I wait until the issue situation is clarified? Cause there are voices shouting against zipfile modification.

Also there's an additional modification that can be done (don't know who's to decide): if some bytes were extracted for the current member, save it and leave it on the disk.

Also If a new PR is to be created against master what happens to this one?

[CristiFati]

@Arszilla: already signed the doc Friday evening (GMT + 2), but I think it takes some time to process the info.

[tirkarthi]

@CristiFati I would recommend waiting on core dev approval. I wrote the comment before the discussion as a general PR workflow to raise it against master since the behavior is present also in master.

[ned-deily]

As noted on the bug issues, it's not clear whether this behavior should be changed at all but, in any case, it won't be for 3.6 which is now in security-fix-only mode. So I'm closing this PR. Sorry!