#1461 Zip service does not preserve all files' permissions

Unix::Linux
closed-fixed
Sharon Lucas
5
2012-06-01
2012-05-29
asomers
No

When zipping multiple files with the execute bit set, and subsequently unzipping them with the "restorepermissions" option, one of the files will have its permissions restored to 666 instead of the correct value. I can't figure out the exact conditions that will trigger this bug, but I've written a testcase based on my actual use. The contents of the files have been deleted, but the bug still triggers.

To reproduce:
1) extract restorepermissions_bug.tar.gz to any directory
2) cd to that directory
3) ./regression.sh
4) Look at the output. The desired output is that all files should have the same permissions as their sources. But at least one (on my system, regression.sh) will have 666 instead.

zBuild:/tmp/ziptest_source # uname -a
Linux zBuild 2.6.34.7-0.7-xen #1 SMP 2010-12-13 11:13:53 +0100 x86_64 x86_64 x86_64 GNU/Linux
zBuild:/tmp/ziptest_source # staf local misc version
Response
--------
3.4.8
zBuild:/tmp/ziptest_source # staf local zip version
Response
--------
3.4.0

Discussion

1 2 > >> (Page 1 of 2)
  • Sharon Lucas
    Sharon Lucas
    2012-05-29

    This restore permissions problem only occurs on systems using Big Endian (e.g. zLinux, Solaris Sparc) and does not occur on Little Endian systems (e.g. Intel Linux i386, Solaris x86). The ZIP service is using a shift operation when obtaining the file attributes which only works correctly on Little Endian systems in the way it is currently implemented. I will look into fixing this. Thank you for reporting this bug.

    "Little Endian" means that the lower order byte of the number is stored in memory at the lowest address, and the higher order byte is stored at the highest address. That is, the little end comes first.

    Intel processors use "Little Endian" byte order.

    "Big Endian" means that the higher order byte of the number is stored in memory at the lowest address, and the lower order byte at the highest address. The big end comes first.

    Motorola, Solaris processors use "Big Endian" byte order.

     
  • Sharon Lucas
    Sharon Lucas
    2012-05-29

    • milestone: --> 2928762
    • assigned_to: nobody --> slucas
     
  • asomers
    asomers
    2012-05-29

    Sharon, I think you must have found a different problem. I am experiencing the problem when compressing on Linux x86_64 under Xen and decompressing either on the same platform or on FreeBSD x86_64 under Xen.

     
  • Sharon Lucas
    Sharon Lucas
    2012-05-29

    Operating systems and hardware together determine endian-ness. You need consider both, as various processors can operate in either little- or big-endian byte order.

    What is the output of the following command when run on a system which is experiencing the restore permissions problem?

    echo -n I | od -to2 | head -n1 | cut -f2 -d" " | cut -c6

    If it outputs 0, it is using "Big Endian". If it outputs 1, it is using "Little Endian".

     
  • asomers
    asomers
    2012-05-29

    spectra@zBuild:/tmp> echo -n I | od -to2 | head -n1 | cut -f2 -d" " | cut -c6
    1

     
  • Sharon Lucas
    Sharon Lucas
    2012-05-30

    Ok, in looking at it further, it appears this issue only happens on 64-bit systems when using a 64-bit version of STAF (not on 32-bit systems) and appears to cause the last file in the zip file's central directory (stored internally within the zip file) to not have its permissions restored which results in its permissions being set to the default 666 (-rw-rw-rw-) instead of the correct permissions for this file (-r-xr-xr-x). I need to investigate further to determine what is causing this issue.

     
  • Sharon Lucas
    Sharon Lucas
    2012-05-31

    Determined that the problem is in the readInData() method in services/zip/unix/STAFZipCentralDirExtension.cpp.

    In the loop where it reads the central dir extension block, at the end of the loop, it is incorrectly decrementing the ul counter which is used to determine if the end of the central dir extension block has been reached. To reduce the size counter (ul) by one record within this loop, instead of doing:

    ul -= 2 + fa->filename_length +
    sizeof(uLong) + sizeof(uLong) + sizeof(uLong);

    I changed it to:

    ul -= sizeof(unsigned short) + fa->filename_length +
    sizeof(mode_t) + sizeof(uid_t) + sizeof(gid_t);

    This way, the size of the mode, owner, and group fields is now correct on both 32-bit and 64-bit systems.. Using sizeof(uLong) on 32-bit systems worked because it is 4. However, on 64-bit s ystems, sizeof(uLong) is usually 8. Thus, it could think it was done reading the central directory extension block when it wasn't (depending on the # of entries in it and the length of the file names) and this would result in the file permissions not being updated for the missed entry(s).

    This fix will be in the next release of STAF (planned for the end of June 2012).

    I plan to check in this fix tomorrow. If you build STAF yourself, you can get the updated services/zip/unix/STAFZipCentralDirExtension.cpp file. Otherwise, if you would like a private fix now, let me know for which operating system(s) and I'll provide updated ZIP service binay(s).

     
  • Sharon Lucas
    Sharon Lucas
    2012-05-31

    • milestone: 2928762 --> Unix::Linux
     
  • Sharon Lucas
    Sharon Lucas
    2012-06-01

    After doing more testing, it looks like the actual length of these fields need to be hardcoded as sizeof(mode_t), sizeof(uid_t), sizeof(gid_t), etc. varies depending on the operating system and architecture. So, need to do:

    ul -= 2 + fa->filenamd_length + 4 + 4 + 4;

    I'm testing on all platforms now to verify.

     
1 2 > >> (Page 1 of 2)