tdf#154587: allow directory entries in ZIP packages

The problem in the bugdoc was the directory entries. These entries
are valid in ZIP packages (even if not common); they may be useful
to e.g. define per-directory permissions (ACLs).

In normal mode, ZipFile reads central directory; there we can read
if the entry has FAT file attributes; and then, if the entry is a
directory. Then it is OK to skip it.

In repair mode, central directory is not used, local file headers
don't contain a "directory" flag. A workaround is used, checking
if there are entries that represent directories of other entries.

Also this change fixes some places that didn't pass the recovery
flag correctly.

Change-Id: I324671841a2c4d0f279b03801d95c8f2eeb99b46
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162888
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

1 files changed, 40 insertions, 2 deletions

diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index 474b73ff53db..71fd66f08196 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -38,6 +38,7 @@
 #include <rtl/digest.h>
 #include <sal/log.hxx>
 #include <o3tl/safeint.hxx>
+#include <o3tl/string_view.hxx>
 #include <osl/diagnose.h>
 
 #include <algorithm>
@@ -1073,7 +1074,7 @@ sal_Int32 ZipFile::readCEN()
             if ( nTestSig != CENSIG )
                 throw ZipException("Invalid CEN header (bad signature)" );
 
-            aMemGrabber.skipBytes ( 2 );
+            sal_uInt16 versionMadeBy = aMemGrabber.ReadUInt16();
             aEntry.nVersion = aMemGrabber.ReadInt16();
             aEntry.nFlag = aMemGrabber.ReadInt16();
 
@@ -1093,7 +1094,8 @@ sal_Int32 ZipFile::readCEN()
             aEntry.nPathLen = aMemGrabber.ReadInt16();
             aEntry.nExtraLen = aMemGrabber.ReadInt16();
             nCommentLen = aMemGrabber.ReadInt16();
-            aMemGrabber.skipBytes ( 8 );
+            aMemGrabber.skipBytes ( 4 );
+            sal_uInt32 externalFileAttributes = aMemGrabber.ReadUInt32();
             sal_uInt64 nOffset = aMemGrabber.ReadUInt32();
 
             if ( aEntry.nPathLen < 0 )
@@ -1132,6 +1134,15 @@ sal_Int32 ZipFile::readCEN()
                 throw ZipException("Integer-overflow");
 
             aMemGrabber.skipBytes(nCommentLen);
+
+            // Is this a FAT-compatible empty entry?
+            if (aEntry.nSize == 0 && (versionMadeBy & 0xff00) == 0)
+            {
+                constexpr sal_uInt32 FILE_ATTRIBUTE_DIRECTORY = 16;
+                if (externalFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+                    continue; // This is a directory entry, not a stream - skip it
+            }
+
             aEntries[aEntry.sPath] = aEntry;
         }
 
@@ -1253,6 +1264,7 @@ void ZipFile::recover()
                                                               RTL_TEXTENCODING_UTF8 );
                                     aEntry.nPathLen = static_cast< sal_Int16 >(aFileName.getLength());
                                 }
+                                aEntry.sPath = aEntry.sPath.replace('\\', '/');
 
                                 // read 64bit header
                                 if (aEntry.nExtraLen > 0)
@@ -1294,7 +1306,33 @@ void ZipFile::recover()
                                         aEntry.nSize = 0;
                                     }
 
+                                    // Do not add this entry, if it is empty and is a directory of
+                                    // an already existing entry
+                                    if (aEntry.nSize == 0 && aEntry.nCompressedSize == 0
+                                        && std::find_if(
+                                               aEntries.begin(), aEntries.end(),
+                                               [path = OUString(aEntry.sPath + "/")](const auto& r)
+                                               { return r.first.startsWith(path); })
+                                               != aEntries.end())
+                                        continue;
+
                                     aEntries.emplace( aEntry.sPath, aEntry );
+
+                                    // Drop any "directory" entry corresponding to this one's path;
+                                    // since we don't use central directory, we don't see external
+                                    // file attributes, so sanitize here
+                                    sal_Int32 i = 0;
+                                    for (OUString subdir = aEntry.sPath.getToken(0, '/', i); i >= 0;
+                                         subdir += OUString::Concat("/")
+                                                   + o3tl::getToken(aEntry.sPath, 0, '/', i))
+                                    {
+                                        if (auto it = aEntries.find(subdir); it != aEntries.end())
+                                        {
+                                            // if not empty, let it fail later in ZipPackage::getZipFileContents
+                                            if (it->second.nSize == 0 && it->second.nCompressedSize == 0)
+                                                aEntries.erase(it);
+                                        }
+                                    }
                                 }
                             }
                         }