Feature/acs 3163 secure import action against non recursive zip bomb (#1194)

* Compression ratio based zip bomb protection

* Compression ratio based zip bomb protection

* ACS-3163 Add uncompressed bytes limit and tests

* ACS-3163 remove static fields

* remove duplicated code

* Use Null object for empty tracker

Co-authored-by: pzurek <Piotr.Zurek@hyland.com>

repository/src/main/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -64,6 +64,7 @@ import org.alfresco.service.namespace.QName;
 import org.alfresco.util.TempFileProvider;
 import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
 import org.apache.commons.compress.archivers.zip.ZipFile;
+import org.apache.commons.compress.utils.InputStreamStatistics;
 
 /**
  * Importer action executor
@@ -80,9 +81,11 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
     private static final int BUFFER_SIZE = 16384;
     private static final String TEMP_FILE_PREFIX = "alf";
     private static final String TEMP_FILE_SUFFIX_ACP = ".acp";
-    
+
+    private long ratioThreshold;
+    private long uncompressedBytesLimit = -1L;
     private boolean highByteZip = false;
-    
+
     /**
      * The importer service
      */
@@ -159,6 +162,36 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
        this.highByteZip = highByteZip;
     }
 
+    /**
+     * @param ratioThreshold the compression ratio threshold for Zip bomb detection
+     */
+    public void setRatioThreshold(long ratioThreshold)
+    {
+        this.ratioThreshold = ratioThreshold;
+    }
+
+    /**
+     * This method sets a value for the uncompressed bytes limit. If the string does not {@link Long#parseLong(String) parse} to a
+     * java long.
+     *
+     * @param limit a String representing a valid Java long.
+     */
+    public void setUncompressedBytesLimit(String limit)
+    {
+        // A string parameter is used here in order to not to require end users to provide a value for the limit in a property
+        // file. This results in the empty string being injected to this method.
+        long longLimit = -1L;
+        try
+        {
+            longLimit = Long.parseLong(limit);
+        }
+        catch (NumberFormatException ignored)
+        {
+            // Intentionally empty
+        }
+        this.uncompressedBytesLimit = longLimit;
+    }
+
     /**
      * @see org.alfresco.repo.action.executer.ActionExecuter#execute(Action, NodeRef)
      */
@@ -229,7 +262,7 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
                        {
                            // TODO: improve this code to directly pipe the zip stream output into the repo objects - 
                            //       to remove the need to expand to the filesystem first?
-                           extractFile(zipFile, tempDir.getPath());
+                           extractFile(zipFile, tempDir.getPath(), new ZipBombProtection(ratioThreshold, uncompressedBytesLimit));
                            importDirectory(tempDir.getPath(), importDest);
                        }
                        finally
@@ -338,14 +371,26 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
         paramList.add(new ParameterDefinitionImpl(PARAM_ENCODING, DataTypeDefinition.TEXT, 
               false, getParamDisplayLabel(PARAM_ENCODING)));
     }
-    
+
     /**
      * Extract the file and folder structure of a ZIP file into the specified directory
-     * 
+     *
      * @param archive       The ZIP archive to extract
      * @param extractDir    The directory to extract into
      */
     public static void extractFile(ZipFile archive, String extractDir)
+    {
+        extractFile(archive, extractDir, ExtractionProgressTracker.NONE);
+    }
+
+    /**
+     * Extract the file and folder structure of a ZIP file into the specified directory using a progress tracker
+     *
+     * @param archive       The ZIP archive to extract
+     * @param extractDir    The directory to extract into
+     * @param tracker       The extraction progress tracker to check against during the extraction process
+     */
+    public static void extractFile(ZipFile archive, String extractDir, ExtractionProgressTracker tracker)
     {
         String fileName;
         String destFileName;
@@ -353,6 +398,9 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
         extractDir = extractDir + File.separator;
         try
         {
+            long totalCompressedBytesCount = 0;
+            long totalUncompressedBytesCount = 0;
+            tracker.reportProgress(0, 0);
             for (Enumeration<ZipArchiveEntry> e = archive.getEntries(); e.hasMoreElements();)
             {
                 ZipArchiveEntry entry = e.nextElement();
@@ -374,15 +422,21 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
                         File parentFile = new File(parent);
                         if (!parentFile.exists()) parentFile.mkdirs();
                     }
-                    InputStream in = new BufferedInputStream(archive.getInputStream(entry), BUFFER_SIZE);
-                    OutputStream out = new BufferedOutputStream(new FileOutputStream(destFileName), BUFFER_SIZE);
-                    int count;
-                    while ((count = in.read(buffer)) != -1)
+
+                    try (InputStream zis = archive.getInputStream(entry);
+                         InputStream in = new BufferedInputStream(zis, BUFFER_SIZE);
+                         OutputStream out = new BufferedOutputStream(new FileOutputStream(destFileName), BUFFER_SIZE))
                     {
-                        out.write(buffer, 0, count);
+                        final InputStreamStatistics entryStats = (InputStreamStatistics) zis;
+                        int count;
+                        while ((count = in.read(buffer)) != -1)
+                        {
+                            tracker.reportProgress(totalCompressedBytesCount + entryStats.getCompressedCount(), totalUncompressedBytesCount + entryStats.getUncompressedCount());
+                            out.write(buffer, 0, count);
+                        }
+                        totalCompressedBytesCount += entryStats.getCompressedCount();
+                        totalUncompressedBytesCount += entryStats.getUncompressedCount();
                     }
-                    in.close();
-                    out.close();
                 }
                 else
                 {
@@ -432,4 +486,51 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase
             dir.delete();
         }
     }
+
+    private static class ZipBombProtection implements ExtractionProgressTracker
+    {
+        private final long ratioThreshold;
+        private final long uncompressedBytesLimit;
+
+        private ZipBombProtection(long ratioThreshold, long uncompressedBytesLimit)
+        {
+            this.ratioThreshold = ratioThreshold;
+            this.uncompressedBytesLimit = uncompressedBytesLimit;
+        }
+
+        @Override
+        public void reportProgress(long compressedBytesCount, long uncompressedBytesCount)
+        {
+            if (compressedBytesCount <= 0 || uncompressedBytesCount <= 0)
+            {
+                return;
+            }
+
+            long ratio = uncompressedBytesCount / compressedBytesCount;
+
+            if (ratio > ratioThreshold)
+            {
+                throw new AlfrescoRuntimeException("Unexpected compression ratio detected (" + ratio + "%). Possible zip bomb attack. Breaking the extraction process.");
+            }
+
+            if (uncompressedBytesLimit > 0 && uncompressedBytesCount > uncompressedBytesLimit)
+            {
+                throw new AlfrescoRuntimeException("Uncompressed bytes limit exceeded (" + uncompressedBytesCount + "). Possible zip bomb attack. Breaking the extraction process.");
+            }
+        }
+    }
+
+    private interface ExtractionProgressTracker
+    {
+        void reportProgress(long compressedBytesCount, long uncompressedBytesCount);
+
+        ExtractionProgressTracker NONE = new ExtractionProgressTracker()
+        {
+            @Override
+            public void reportProgress(long compressedBytesCount, long uncompressedBytesCount)
+            {
+                // intentionally do nothing
+            }
+        };
+    }
 }

repository/src/main/resources/alfresco/action-services-context.xml

@@ -593,6 +593,8 @@
         <property name="fileFolderService">
             <ref bean="FileFolderService"/>
         </property>
+       <property name="ratioThreshold" value="${import.zip.compressionRatioThreshold}"/>
+       <property name="uncompressedBytesLimit" value="${import.zip.uncompressedBytesLimit}"/>
     </bean>
     
     <bean id="export" class="org.alfresco.repo.action.executer.ExporterActionExecuter" parent="action-executer">

repository/src/main/resources/alfresco/repository.properties

@@ -1336,3 +1336,11 @@ allow.unsecure.callback.jsonp=false
 
 # pre-configured allow list of media/mime types to allow inline instead of attachment (via Content-Disposition response header)
 content.nonAttach.mimetypes=application/pdf,image/jpeg,image/gif,image/png,image/tiff,image/bmp
+
+# Zip file compression ratio threshold as a percentage, above which the zip file will be considered a "zip bomb" and the
+# import extraction process cancelled.
+import.zip.compressionRatioThreshold=100
+# Maximum number of uncompressed bytes allowed for an imported zip file above which the zip file will be considered a
+# "zip bomb" and the import extraction process cancelled. No value (or a negative long) will be taken to mean that no
+# limit should be applied.
+import.zip.uncompressedBytesLimit=

repository/src/test/java/org/alfresco/AllUnitTestsSuite.java

@@ -176,6 +176,7 @@ import org.junit.runners.Suite;
     org.alfresco.repo.action.CompositeActionImplTest.class,
     org.alfresco.repo.action.CompositeActionConditionImplTest.class,
     org.alfresco.repo.action.executer.TransformActionExecuterTest.class,
+    org.alfresco.repo.action.executer.ImporterActionExecutorUnitTest.class,
     org.alfresco.repo.audit.AuditableAnnotationTest.class,
     org.alfresco.repo.audit.PropertyAuditFilterTest.class,
     org.alfresco.repo.audit.access.NodeChangeTest.class,

repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -214,6 +214,98 @@ public class ImporterActionExecuterTest
         });
     }
 
+    /**
+     * Test zip file that exceeds the compression ratio threshold (non-recursive zip bomb)
+     *
+     * @throws IOException
+     */
+    @Test
+    public void testImportThatExceedsCompressionRatioFails() throws IOException
+    {
+        final RetryingTransactionHelper retryingTransactionHelper = serviceRegistry.getRetryingTransactionHelper();
+
+        retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            public Void execute()
+            {
+                NodeRef rootNodeRef = nodeService.getRootNode(storeRef);
+
+                // create test data
+                NodeRef zipFileNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_CONTENT).getChildRef();
+                NodeRef targetFolderNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_FOLDER).getChildRef();
+
+                putContent(zipFileNodeRef, "import-archive-test/exceedsRatio.zip");
+
+                Action action = createAction(zipFileNodeRef, "ImporterActionExecuterTestActionDefinition", targetFolderNodeRef);
+
+                try
+                {
+                    importerActionExecuter.setRatioThreshold(3);
+                    importerActionExecuter.execute(action, zipFileNodeRef);
+                    fail("Import action should have detected that zip file exceeds compression ration threshold");
+                }
+                catch (AlfrescoRuntimeException e)
+                {
+                    assertTrue(e.getMessage().contains("Unexpected compression ratio detected"));
+                }
+                finally
+                {
+                    // clean test data
+                    nodeService.deleteNode(targetFolderNodeRef);
+                    nodeService.deleteNode(zipFileNodeRef);
+                }
+
+                return null;
+            }
+        });
+    }
+
+    /**
+     * Test zip file that exceeds the uncompressed bytes limit
+     *
+     * @throws IOException
+     */
+    @Test
+    public void testImportThatExceedsUncompressedBytesLimitFails() throws IOException
+    {
+        final RetryingTransactionHelper retryingTransactionHelper = serviceRegistry.getRetryingTransactionHelper();
+
+        retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            public Void execute()
+            {
+                NodeRef rootNodeRef = nodeService.getRootNode(storeRef);
+
+                // create test data
+                NodeRef zipFileNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_CONTENT).getChildRef();
+                NodeRef targetFolderNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_FOLDER).getChildRef();
+
+                putContent(zipFileNodeRef, "import-archive-test/exceedsUncompressedBytesLimit.zip");
+
+                Action action = createAction(zipFileNodeRef, "ImporterActionExecuterTestActionDefinition", targetFolderNodeRef);
+
+                try
+                {
+                    importerActionExecuter.setUncompressedBytesLimit("1000");
+                    importerActionExecuter.execute(action, zipFileNodeRef);
+                    fail("Import action should have detected that uncompressed bytes exceed limit");
+                }
+                catch (AlfrescoRuntimeException e)
+                {
+                    assertTrue(e.getMessage().contains("Uncompressed bytes limit exceeded"));
+                }
+                finally
+                {
+                    // clean test data
+                    nodeService.deleteNode(targetFolderNodeRef);
+                    nodeService.deleteNode(zipFileNodeRef);
+                }
+
+                return null;
+            }
+        });
+    }
+
     private void putContent(NodeRef zipFileNodeRef, String resource)
     {
         URL url = AbstractContentTransformerTest.class.getClassLoader().getResource(resource);

repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecutorUnitTest.java

@@ -0,0 +1,76 @@
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software.
+ * If the software was purchased under a paid Alfresco license, the terms of
+ * the paid license agreement will prevail.  Otherwise, the software is
+ * provided under the following open source license terms:
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.repo.action.executer;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.alfresco.util.GUID;
+import org.alfresco.util.TempFileProvider;
+import org.apache.commons.compress.archivers.zip.ZipFile;
+import org.junit.Test;
+
+/**
+ * Unit tests for import action executor.
+ *
+ */
+public class ImporterActionExecutorUnitTest
+{
+    private static final String TEST_RESOURCE_PATHNAME = "import-archive-test";
+
+    @Test
+    public void testExtractFileDoesNotFailWhenCalledWithNoTracker() throws Exception
+    {
+        // create test folder in which to extract the zip file
+        final File destinationDir = TempFileProvider.getTempDir(GUID.generate());
+
+        // zip file to be extracted
+        final ZipFile zipFile = getZipResource("exceedsRatio.zip");
+
+        // extract the zip file to the destination folder
+        assertTrue("Expected destination folder to be empty.", destinationDir.list().length == 0);
+        ImporterActionExecuter.extractFile(zipFile, destinationDir.getPath());
+        assertTrue("Expected destination folder to contain at least one file.", destinationDir.list().length > 0);
+
+        ImporterActionExecuter.deleteDir(destinationDir);
+    }
+
+    @SuppressWarnings("SameParameterValue")
+    private ZipFile getZipResource(String zipFileName) throws URISyntaxException, IOException
+    {
+        final String zipFilePath = TEST_RESOURCE_PATHNAME + File.separator + zipFileName;
+        final URL url = ImporterActionExecutorUnitTest.class.getClassLoader().getResource(zipFilePath);
+        final Path path = Paths.get(url.toURI());
+        final File file = new File(path.toString());
+        return new ZipFile(file, "UTF-8", true);
+    }
+}

repository/src/test/resources/alfresco-global.properties

@@ -64,4 +64,5 @@ encryption.keystore.keyMetaData.location=${dir.keystore}/keystore-passwords.prop
 encryption.keyAlgorithm=DESede
 encryption.cipherAlgorithm=DESede/CBC/PKCS5Padding
 encryption.keystore.type=JCEKS
-encryption.keystore.backup.type=JCEKS
+encryption.keystore.backup.type=JCEKS
+