From cdefbd2bf16590b6c0e986430e930f1b5e177345 Mon Sep 17 00:00:00 2001 From: Alex Mukha Date: Mon, 9 Nov 2015 11:51:52 +0000 Subject: [PATCH] Merged 5.1.N (5.1.1) to HEAD (5.1) 116289 amukha: ACE-4513: Merged 5.0.N (5.0.3) to 5.1.N (5.1.1) 116284 amukha: MNT-15091: Merged V4.2-BUG-FIX (4.2.6) to 5.0.N (5.0.3) 116280 amukha: MNT-15090: Merged DEV to V4.2-BUG-FIX (4.2.6) 115799: MNT-15075 : [Pentest 121015] ZIP extraction code execution Fix and test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@116583 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../executer/ImporterActionExecuter.java | 7 ++ .../alfresco/repo/action/ActionTestSuite.java | 4 +- .../executer/ImporterActionExecuterTest.java | 119 ++++++++++++++++++ .../SuspiciousPathsArchive.zip | Bin 0 -> 1286 bytes 4 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 source/test-java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java create mode 100644 source/test-resources/import-archive-test/SuspiciousPathsArchive.zip diff --git a/source/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java b/source/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java index 5d46ff5fbf..c47e36d7bc 100644 --- a/source/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java +++ b/source/java/org/alfresco/repo/action/executer/ImporterActionExecuter.java @@ -69,6 +69,7 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase public static final String NAME = "import"; public static final String PARAM_ENCODING = "encoding"; public static final String PARAM_DESTINATION_FOLDER = "destination"; + public static final String ARCHIVE_CONTAINS_SUSPICIOUS_PATHS_ERROR = "Archive contains suspicious paths. Please review it's contents and make sure it doesn't contain entries with absolute paths or paths containing references to the parent folder (i.e. \"..\")"; private static final int BUFFER_SIZE = 16384; private static final String TEMP_FILE_PREFIX = "alf"; @@ -348,6 +349,12 @@ public class ImporterActionExecuter extends ActionExecuterAbstractBase { fileName = entry.getName(); fileName = fileName.replace('/', File.separatorChar); + + if (fileName.startsWith("/") || fileName.indexOf(":" + File.separator) == 1 || fileName.contains(".." + File.separator)) + { + throw new AlfrescoRuntimeException(ARCHIVE_CONTAINS_SUSPICIOUS_PATHS_ERROR); + } + destFileName = extractDir + fileName; File destFile = new File(destFileName); String parent = destFile.getParent(); diff --git a/source/test-java/org/alfresco/repo/action/ActionTestSuite.java b/source/test-java/org/alfresco/repo/action/ActionTestSuite.java index 383512eaa0..28a41fc511 100644 --- a/source/test-java/org/alfresco/repo/action/ActionTestSuite.java +++ b/source/test-java/org/alfresco/repo/action/ActionTestSuite.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2013 Alfresco Software Limited. + * Copyright (C) 2005-2015 Alfresco Software Limited. * * This file is part of Alfresco * @@ -25,6 +25,7 @@ import org.alfresco.repo.action.evaluator.IsSubTypeEvaluatorTest; import org.alfresco.repo.action.executer.AddFeaturesActionExecuterTest; import org.alfresco.repo.action.executer.ContentMetadataEmbedderTest; import org.alfresco.repo.action.executer.ContentMetadataExtracterTest; +import org.alfresco.repo.action.executer.ImporterActionExecuterTest; import org.alfresco.repo.action.executer.MailActionExecuterTest; import org.alfresco.repo.action.executer.RemoveFeaturesActionExecuterTest; import org.alfresco.repo.action.executer.SetPropertyValueActionExecuterTest; @@ -67,6 +68,7 @@ import org.junit.runners.Suite.SuiteClasses; ActionTrackingServiceImplTest.class, // intermittent - pending ALF-9773 & ALF-9774 MailActionExecuterTest.class, ActionServiceImpl2Test.class, + ImporterActionExecuterTest.class }) public class ActionTestSuite { diff --git a/source/test-java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java b/source/test-java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java new file mode 100644 index 0000000000..a78736ceda --- /dev/null +++ b/source/test-java/org/alfresco/repo/action/executer/ImporterActionExecuterTest.java @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2005-2015 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * 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 . + */ +package org.alfresco.repo.action.executer; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.File; +import java.io.IOException; + +import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.model.ContentModel; +import org.alfresco.repo.action.ActionImpl; +import org.alfresco.repo.content.MimetypeMap; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; +import org.alfresco.service.cmr.action.Action; +import org.alfresco.service.cmr.repository.ContentData; +import org.alfresco.service.cmr.repository.ContentService; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.cmr.repository.StoreRef; +import org.alfresco.service.namespace.QName; +import org.alfresco.util.GUID; +import org.alfresco.util.test.junitrules.ApplicationContextInit; +import org.junit.Test; + +/** + * This class contains tests for {@link ImporterActionExecuter}. + * + * @author abalmus + */ +public class ImporterActionExecuterTest +{ + @Test + public void testImportArchiveWithSuspiciousPaths() throws IOException + { + final ApplicationContextInit applicationContextInit = new ApplicationContextInit(); + final ServiceRegistry serviceRegistry = (ServiceRegistry) applicationContextInit.getApplicationContext().getBean(ServiceRegistry.SERVICE_REGISTRY); + final NodeService nodeService = serviceRegistry.getNodeService(); + final ContentService contentService = serviceRegistry.getContentService(); + final RetryingTransactionHelper retryingTransactionHelper = serviceRegistry.getTransactionService().getRetryingTransactionHelper(); + + final File file = new File("./source/test-resources/import-archive-test/SuspiciousPathsArchive.zip"); + + retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback() + { + public Void execute() + { + AuthenticationUtil.setRunAsUserSystem(); + + StoreRef storeRef = nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "Test_" + System.nanoTime()); + + NodeRef rootNodeRef = nodeService.getRootNode(storeRef); + + NodeRef zipFileNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, + QName.createQName("http://www.alfresco.org/test/ImporterActionExecuterTest", "testAssocQName1"), + ContentModel.TYPE_CONTENT).getChildRef(); + + NodeRef targetFolderNodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, + QName.createQName("http://www.alfresco.org/test/ImporterActionExecuterTest", "testAssocQName2"), + ContentModel.TYPE_FOLDER).getChildRef(); + + contentService.getWriter(zipFileNodeRef, ContentModel.PROP_CONTENT, true).putContent(file); + + ContentData contentData = (ContentData) nodeService.getProperty(zipFileNodeRef, ContentModel.PROP_CONTENT); + ContentData newContentData = ContentData.setMimetype(contentData, MimetypeMap.MIMETYPE_ZIP); + + nodeService.setProperty(zipFileNodeRef, ContentModel.PROP_CONTENT, newContentData); + + Action action = new ActionImpl(zipFileNodeRef, GUID.generate(), "ImporterActionExecuterTestActionDefinition"); + action.setParameterValue(ImporterActionExecuter.PARAM_DESTINATION_FOLDER, targetFolderNodeRef); + action.setParameterValue(ImporterActionExecuter.PARAM_ENCODING, "UTF-8"); + + ImporterActionExecuter executer = new ImporterActionExecuter(); + executer.setNodeService(nodeService); + executer.setContentService(contentService); + + try + { + executer.execute(action, zipFileNodeRef); + fail("An AlfrescoRuntimeException should have occured."); + } + catch (AlfrescoRuntimeException e) + { + assertTrue(e.getMessage().contains(ImporterActionExecuter.ARCHIVE_CONTAINS_SUSPICIOUS_PATHS_ERROR)); + } + finally + { + nodeService.deleteNode(targetFolderNodeRef); + nodeService.deleteNode(zipFileNodeRef); + nodeService.deleteStore(storeRef); + + AuthenticationUtil.clearCurrentSecurityContext(); + } + + return null; + } + }); + } +} diff --git a/source/test-resources/import-archive-test/SuspiciousPathsArchive.zip b/source/test-resources/import-archive-test/SuspiciousPathsArchive.zip new file mode 100644 index 0000000000000000000000000000000000000000..a0456d3f0f514cc5b77de9ab8e332092651cbf7c GIT binary patch literal 1286 zcmWIWW@Zs#U|`^2i0BJ;f3h|4TP={c7>KnQWEk}H^s%DUl4SklqWnC)6fl*Mn4BFN z!pXqw!K@ml$zd2)TEWf0$dW4_7s&!NiNUq|AlD%S0hjl6uIF0QIvrQHHmUztUKTAK zad*S+NAG8IJvx&(Z_k4d?@u~e_yWtUw zH+BIyeJBR;AwJJAbD?N3W!!1Rg=^L&i@Ea6bW4At7(63_5Qy zGRZOH%3Bhke8Ip7#7i1MEUeiKyRSn-SRpwLEtds&