diff --git a/repository/src/main/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java b/repository/src/main/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java index 97b71f8cb3..d860444267 100644 --- a/repository/src/main/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java +++ b/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 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 + } + }; + } } diff --git a/repository/src/main/resources/alfresco/action-services-context.xml b/repository/src/main/resources/alfresco/action-services-context.xml index 14c3b407ca..632f0707be 100644 --- a/repository/src/main/resources/alfresco/action-services-context.xml +++ b/repository/src/main/resources/alfresco/action-services-context.xml @@ -593,6 +593,8 @@ + + diff --git a/repository/src/main/resources/alfresco/repository.properties b/repository/src/main/resources/alfresco/repository.properties index 26046746d8..7954cff846 100644 --- a/repository/src/main/resources/alfresco/repository.properties +++ b/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= diff --git a/repository/src/test/java/org/alfresco/AllUnitTestsSuite.java b/repository/src/test/java/org/alfresco/AllUnitTestsSuite.java index a956aeb879..54924edee0 100644 --- a/repository/src/test/java/org/alfresco/AllUnitTestsSuite.java +++ b/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, diff --git a/repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java b/repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java index 15c29b67e9..397d74db90 100644 --- a/repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java +++ b/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() + { + 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() + { + 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); diff --git a/repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecutorUnitTest.java b/repository/src/test/java/org/alfresco/repo/action/executer/ImporterActionExecutorUnitTest.java new file mode 100644 index 0000000000..dae86c8afc --- /dev/null +++ b/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 . + * #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); + } +} \ No newline at end of file diff --git a/repository/src/test/resources/alfresco-global.properties b/repository/src/test/resources/alfresco-global.properties index 08a5956690..4e9836dd2d 100644 --- a/repository/src/test/resources/alfresco-global.properties +++ b/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 \ No newline at end of file +encryption.keystore.backup.type=JCEKS + diff --git a/repository/src/test/resources/import-archive-test/exceedsRatio.zip b/repository/src/test/resources/import-archive-test/exceedsRatio.zip new file mode 100644 index 0000000000..4f2bfc9b77 Binary files /dev/null and b/repository/src/test/resources/import-archive-test/exceedsRatio.zip differ diff --git a/repository/src/test/resources/import-archive-test/exceedsUncompressedBytesLimit.zip b/repository/src/test/resources/import-archive-test/exceedsUncompressedBytesLimit.zip new file mode 100644 index 0000000000..3845a0c481 Binary files /dev/null and b/repository/src/test/resources/import-archive-test/exceedsUncompressedBytesLimit.zip differ