Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

64 bits vs 32 bits #45

Closed
magic-tech opened this issue Feb 24, 2016 · 24 comments
Closed

64 bits vs 32 bits #45

magic-tech opened this issue Feb 24, 2016 · 24 comments
Labels
1.2 Related to older version of minizip 1.2 encryption Encryption/decryption issue fixed Issue or bug has been fixed

Comments

@magic-tech
Copy link

Hi, I'm using your lib in my game engine, and yesterday i have tried to unzip a file generated on macosx (64) on windows 10 (64), but for my surprise it can't open even in winrar, and the file generated on windows doesn't open on linux (64) or macosx (64)...
So i don't know if its a os problem or architecture.
If you guys wanna test it, here are the links:
https://www.dropbox.com/s/l1h1zz5n9ivkdup/data.magic3d
https://www.dropbox.com/s/5s4dlp0qvmytwfd/data.magic3d_windows

My project is https://github.com/magic-tech/Magic3D.git and it is compiled with QT 5.5.1 using mingw32
Thanks in advance.

@nmoinvaz
Copy link
Member

Please uncomment unzip.c @ 534 and see if that fixes it for you.

@magic-tech
Copy link
Author

Unfortunately nothing changed.

If you try open the file, the function unzReadCurrentFile int unzip.c @ 1315 will return -3.

I forgot to mention that I'm using password with those files, but without AES, any problems with it?

@nmoinvaz
Copy link
Member

Since I don't have the password, I can't test extracting the contents. But I did use miniunz to list the contents of data.magic3d and there were no errors.

@magic-tech
Copy link
Author

Sorry, my bad... the password is "magic3d", and listing files works fine... the problem is extract.

@nmoinvaz
Copy link
Member

Something appears to be wrong with the archive or the password. WinRAR can't extract it either when I rename it to .zip.

@magic-tech
Copy link
Author

Nice... so the problem is with the archive or the password, nothing is wrong with the library that i used to create it. Just remembering that both files was crated using minizip lib, just compiled with different compilers and operational systems, and the file https://www.dropbox.com/s/5s4dlp0qvmytwfd/data.magic3d_windows can be extracted with winrar without problems, but the other one created on MACOSX cant be extracted on windows, but can be extracted without problems on MACOS and LINUX. Thanks for the support!

@nmoinvaz
Copy link
Member

I didn't understand before that the minizip library was used to create both archives. I don't have a mac readily available to be able to debug the issue.

@nmoinvaz nmoinvaz reopened this Feb 28, 2016
@fungos
Copy link

fungos commented Feb 29, 2016

@nmoinvaz Saying that something appears to be wrong with the archive only proves the bug in minizip, so you may be really interested in this bug report as it will certainly help improve minizip quality.

You can easily debug this on windows using the file "data.magic" (the one that was generated on a mac, no need to you have a mac, just a working debugger).

I've tested it here on my side with both WinRAR 5.00 (64-bit) and 7-Zip 15.14 (64-bit). The results on my windows 64-bits:

  • 7-Zip was able to correctly open (list files) and extract BOTH files with the password "magic3d";
  • WinRAR was able to open (list files) of both files BUT was unable to extract the file "data.magic3d" with the same password "magic3d". The file "data.magic3d_windows" was correctly extracted though.

From what I've debugged, the problem happens here, put a breakpoint there and compare the execution of unzipping both files, you can see that for some reason s->keys and mostly bizarre the s->pcrc_32_tab are fucked up. This seems to be an issue of bad 32bits<>64bits compatibility.

Hope that this is enough information to help you verify that there is an actual bug in the code and help you get it fixed.

Thank you.

@fungos
Copy link

fungos commented Feb 29, 2016

Not surprisingly the latest closed issue reported by @TobiasWGDE seems amazingly similar to this one that may raise a red flag.

@nmoinvaz
Copy link
Member

Maybe I am doing something wrong but I am unable to extract the archive with 7-zip 15.14 64-bit using data.magic3d and the password "magic3d". The user interface may say 100% complete, but it shows errors in the window below, "Wrong password..". Also 7-zip does something which is create 0KB files when it cannot unzip, so it may look like the files were extracted on disk but they are empty.

capture

The code you highlighted, I went and looked at the source code for 7-zip and also for InfoZIP and they are using very similar code. Thanks for helping me look into this issue.

@nmoinvaz
Copy link
Member

I was able to reproduce the issue using x64 project configuration for minizip.exe and win32 project configuration for miniunz.exe. I created a password protected file with minizip and tried to extract it using miniunz and it failed. When I used both x64 project configurations it worked.

@nmoinvaz
Copy link
Member

Actually I was debugging incorrectly. It doesn't matter if the zip file was created with win32 project configuration or x64 configuration, everything works. It is probably an platform issue.

@fungos
Copy link

fungos commented Feb 29, 2016

That is right, I've saw the files and assumed they worked, but yes even 7-zip is not able to extract it.
Nevertheless I was able to extract this same file in a Linux Mint VirtualBox instance, so the issue may well be something related to platform or even compiler.

@luciocosmo
Copy link
Contributor

Hi all, i have the same exact problem. I can give much more details.

The application which uses minizip works correctly with the same zip archives with password (no aes) generated one year ago. The system unzip utility extracts the files with password correctly on all platform.

Now we switched to ios-arm64 builds, the zips without password works correctly, the zip with passwords gives errors reading files with unzReadCurrentFile (no errors enumerating files).

The same exact problem can be found recompiling on CentOS for 64bit.

I'm using now the current minizip branch, nothing changes.

I'm pretty sure the problem is the implementation in crypt.h, which is called through zdecode macro.

I see as incorrectly ported the functions decrypt_byte(), and maybe update_keys().

static int decrypt_byte(unsigned long* pkeys)
{
unsigned temp; /* POTENTIAL BUG: temp*(temp^1) may overflow in an
* unpredictable manner on 16-bit systems; not a problem
* with any known compiler so far, though */

temp = ((unsigned)(*(pkeys+2)) & 0xffff) | 2;
return (int)(((temp * (temp ^ 1)) >> 8) & 0xff);

}

I tried some adjustments, but I was not lucky.
If anyone have some suggestion I can easily try.

@luciocosmo
Copy link
Contributor

Hi All, see my patch. In my tests fixes the above problem.

@fungos
Copy link

fungos commented Mar 9, 2016

@luciocosmo your patch has too much spacing issues. it is very hard to see what was changed (mainly unzip.c), what was changed in it that is related to this bug?

@luciocosmo
Copy link
Contributor

Ooops, let me fix it.

Il giorno 09/mar/2016, alle ore 03:03, Danny Angelo Carminati Grein notifications@github.com ha scritto:

@luciocosmo your patch has too much spacing issues. it is very hard to see what was changed (mainly unzip.c), what was changed in it that is related to this bug?


Reply to this email directly or view it on GitHub.

@luciocosmo
Copy link
Contributor

It seems I cant win, my editor does tab substitution.
I’ll retry later today.

What’s better ? I delete myfork ?

Il giorno 09/mar/2016, alle ore 03:03, Danny Angelo Carminati Grein notifications@github.com ha scritto:

@luciocosmo your patch has too much spacing issues. it is very hard to see what was changed (mainly unzip.c), what was changed in it that is related to this bug?


Reply to this email directly or view it on GitHub.

@fungos
Copy link

fungos commented Mar 10, 2016

No, what is your editor? It may have a config to change tabs<>spaces. Changing it and saving the may fix, then commit over and it should possible to generate a clean patch.

UPDATE: Ok, I saw you've fixed it!

@magic-tech
Copy link
Author

Nice job @luciocosmo apparently your patch worked... I've tested it and worked both ways... files created on windows were extracted on mac, and files created on mac were extracted on windows.

Unfortunately the files created on MAC before the patch doesn't work anymore (but no problems, the important thing is that now they work everywhere).

If you guys want to try it the files are: (password magic3d)
https://dl.dropboxusercontent.com/u/21202284/data.magic3d_mac
https://dl.dropboxusercontent.com/u/21202284/data.magic3d_windows

@luciocosmo
Copy link
Contributor

Il giorno 11/mar/2016, alle ore 05:35, MagicTech notifications@github.com ha scritto:

Nice job @luciocosmo apparently your patch worked... I've tested it and worked both ways... files created on windows were extracted on mac, and files created on mac were extracted on windows.

Unfortunately the files created on MAC before the patch doesn't work anymore (but no problems, the important thing is that now they work everywhere).

Hi nmoinvaz

Wait wait. In my case the change was absolutely necessary to open MANY libraries files from an application which runs on Android and iOS.
GBytes of files were generated on OSX with a tool which embeds minizip (previous version).

So in my case all is working now correctly.

I hope really no problems come out but i tested on a part of the content and it’s working.
To tell the truth I did not still check recompiling for Android… but long and in on 32 bit are the same :| i really not forsee problems :|

L

@magic-tech
Copy link
Author

Let me explain... the file https://www.dropbox.com/s/l1h1zz5n9ivkdup/data.magic3d that was created with minizip without your patch on MACOS doesn't work after your patch, but without your patch it ONLY works on MACOS, on Windows it returns -3 in the function unzReadCurrentFile.

the file https://www.dropbox.com/s/5s4dlp0qvmytwfd/data.magic3d_windows that was created with minizip without your patch on Windows works fine on Windows but doesn't open on MACOS, it returns -3 in the function unzReadCurrentFile. BUT with you patch it NOW OPENS on MACOS.

AND after your patch all files created (using minizip of course) on MAC or WINDOWS works fine both ways.

Probably before your patch the file https://www.dropbox.com/s/l1h1zz5n9ivkdup/data.magic3d was created wrong because of the size of LONG that may differ on MACOS 64 bits or CLANG may differ from MingW on what happens if you try to convert a LONG to INT and vice versa, and now that is unsigned int the data is write same way on both platforms.

Sorry my poor English and if it is too confuse i can explain again.

@nmoinvaz
Copy link
Member

I have merged the pull request. Thank you everybody! It is crazy to think that something like this has been able to remain for so long without being detected.

@luciocosmo
Copy link
Contributor

Perfectly clear. Makes sense !

Inviato da iPhone

Il giorno 11 mar 2016, alle ore 18:28, MagicTech notifications@github.com ha scritto:

Let me explain... the file https://www.dropbox.com/s/l1h1zz5n9ivkdup/data.magic3d that was created with minizip without your patch on MACOS doesn't work after your patch, but without your patch it ONLY works on MACOS, on Windows it returns -3 in the function unzReadCurrentFile.

the file https://www.dropbox.com/s/5s4dlp0qvmytwfd/data.magic3d_windows that was created with minizip without your patch on Windows works fine on Windows but doesn't open on MACOS, it returns -3 in the function unzReadCurrentFile. BUT with you patch it NOW OPENS on MACOS.

AND after your patch all files created (using minizip of course) on MAC or WINDOWS works fine both ways.

Probably before your patch the file https://www.dropbox.com/s/l1h1zz5n9ivkdup/data.magic3d was created wrong because of the size of LONG that may differ on MACOS 64 bits or CLANG may differ from MingW on what happens if you try to convert a LONG to INT and vice versa, and now that is unsigned int the data is write same way on both platforms.

Sorry my poor English and if it is too confuse i can explain again.


Reply to this email directly or view it on GitHub.

@nmoinvaz nmoinvaz added encryption Encryption/decryption issue fixed Issue or bug has been fixed labels Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.2 Related to older version of minizip 1.2 encryption Encryption/decryption issue fixed Issue or bug has been fixed
Projects
None yet
Development

No branches or pull requests

4 participants