diff --git a/taier-common/src/main/java/com/dtstack/taier/common/util/ZipUtil.java b/taier-common/src/main/java/com/dtstack/taier/common/util/ZipUtil.java index 6c18cf7bff..77d3914f3c 100644 --- a/taier-common/src/main/java/com/dtstack/taier/common/util/ZipUtil.java +++ b/taier-common/src/main/java/com/dtstack/taier/common/util/ZipUtil.java @@ -301,6 +301,7 @@ private static List upzipFile(File zipFile, String descDir, UnzipContext c } private static File resolveZipEntryFile(File baseDir, String basePath, String entryName) throws IOException { + validateZipEntryName(entryName); File targetFile = new File(baseDir, entryName); String targetPath = targetFile.getCanonicalPath(); if (!targetPath.equals(basePath) && !targetPath.startsWith(basePath + File.separator)) { @@ -309,6 +310,15 @@ private static File resolveZipEntryFile(File baseDir, String basePath, String en return targetFile; } + private static void validateZipEntryName(String entryName) throws IOException { + if (entryName == null + || entryName.startsWith("/") + || entryName.startsWith("\\") + || new File(entryName).isAbsolute()) { + throw new IOException(String.format("zip entry is outside of target dir: %s", entryName)); + } + } + private static String getCanonicalDirPath(File dir) throws IOException { makeDirs(dir); return dir.getCanonicalPath(); diff --git a/taier-common/src/test/java/com/dtstack/taier/common/util/ZipUtilTest.java b/taier-common/src/test/java/com/dtstack/taier/common/util/ZipUtilTest.java index aa9af38047..5ef46388f5 100644 --- a/taier-common/src/test/java/com/dtstack/taier/common/util/ZipUtilTest.java +++ b/taier-common/src/test/java/com/dtstack/taier/common/util/ZipUtilTest.java @@ -68,6 +68,23 @@ public void testRejectZipSlipEntry() throws Exception { Assert.assertFalse(escapedFile.exists()); } + @Test + public void testRejectAbsolutePathZipEntry() throws Exception { + File targetDir = temporaryFolder.newFolder("absolute"); + File escapedFile = temporaryFolder.newFile("absolute-evil.txt"); + Files.delete(escapedFile.toPath()); + File zipFile = temporaryFolder.newFile("absolute.zip"); + writeApacheZip(zipFile, escapedFile.getAbsolutePath(), "evil"); + + try { + ZipUtil.upzipFile(zipFile, targetDir.getAbsolutePath()); + Assert.fail("Absolute path zip entry should be rejected"); + } catch (TaierDefineException e) { + Assert.assertTrue(e.getMessage().contains("outside of target dir")); + } + Assert.assertFalse(escapedFile.exists()); + } + @Test public void testRejectTooManyEntries() throws Exception { File zipFile = temporaryFolder.newFile("too-many.zip"); @@ -129,6 +146,15 @@ private static void writeZip(File zipFile, ZipItem... items) throws IOException } } + private static void writeApacheZip(File zipFile, String entryName, String content) throws IOException { + try (org.apache.tools.zip.ZipOutputStream zipOutputStream = + new org.apache.tools.zip.ZipOutputStream(new FileOutputStream(zipFile))) { + zipOutputStream.putNextEntry(new org.apache.tools.zip.ZipEntry(entryName)); + zipOutputStream.write(content.getBytes(StandardCharsets.UTF_8)); + zipOutputStream.closeEntry(); + } + } + private static class ZipItem { private final String name; private final String content; diff --git a/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleClusterService.java b/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleClusterService.java index f5bb7ff438..3082dbed4c 100644 --- a/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleClusterService.java +++ b/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleClusterService.java @@ -36,9 +36,11 @@ import com.dtstack.taier.scheduler.service.ComponentService; import com.dtstack.taier.scheduler.vo.ComponentVO; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import java.io.File; import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; @@ -65,6 +67,7 @@ public class ConsoleClusterService { private ComponentService componentService; public Long addCluster(String clusterName) { + checkClusterName(clusterName); if (clusterMapper.getByClusterName(clusterName) != null) { throw new TaierDefineException(ErrorCode.NAME_ALREADY_EXIST.getDescription()); } @@ -74,6 +77,17 @@ public Long addCluster(String clusterName) { return cluster.getId(); } + private void checkClusterName(String clusterName) { + if (StringUtils.isBlank(clusterName) + || clusterName.contains("/") + || clusterName.contains("\\") + || clusterName.contains("..") + || clusterName.indexOf('\0') >= 0 + || new File(clusterName).isAbsolute()) { + throw new TaierDefineException("Invalid cluster name"); + } + } + public IPage pageQuery(int currentPage, int pageSize) { Page page = new Page<>(currentPage, pageSize); return clusterMapper.selectPage(page, Wrappers.lambdaQuery(Cluster.class).eq( diff --git a/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleComponentService.java b/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleComponentService.java index c6a5826791..53ab011c78 100644 --- a/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleComponentService.java +++ b/taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleComponentService.java @@ -112,6 +112,8 @@ public class ConsoleComponentService { private static final Logger LOGGER = LoggerFactory.getLogger(ComponentService.class); + private static final String LOCAL_KERBEROS_CLUSTER_DIR_PREFIX = "CLUSTER_"; + @Autowired private ComponentMapper componentMapper; @@ -491,7 +493,7 @@ private String updateComponentKerberosFile(Long clusterId, Component addComponen //删除本地文件夹 String kerberosPath = this.getLocalKerberosPath(clusterId, addComponent.getComponentTypeCode()); try { - FileUtils.deleteDirectory(new File(kerberosPath)); + deleteLocalKerberosDirectory(kerberosPath); } catch (IOException e) { LOGGER.error("delete old kerberos directory {} error", kerberosPath, e); } @@ -614,7 +616,22 @@ public String getLocalKerberosPath(Long clusterId, Integer componentCode) { if (null == one) { throw new TaierDefineException(ErrorCode.CANT_NOT_FIND_CLUSTER); } - return env.getTempDir() + File.separator + one.getClusterName() + File.separator + EComponentType.getByCode(componentCode).name() + File.separator + KERBEROS; + if (StringUtils.isBlank(env.getTempDir())) { + throw new TaierDefineException("Temp dir cannot be empty"); + } + return env.getTempDir() + File.separator + LOCAL_KERBEROS_CLUSTER_DIR_PREFIX + clusterId + + File.separator + EComponentType.getByCode(componentCode).name() + File.separator + KERBEROS; + } + + private void deleteLocalKerberosDirectory(String kerberosPath) throws IOException { + File tempDir = new File(env.getTempDir()).getCanonicalFile(); + File kerberosDir = new File(kerberosPath).getCanonicalFile(); + String tempDirPath = tempDir.getPath(); + String kerberosDirPath = kerberosDir.getPath(); + if (!kerberosDirPath.startsWith(tempDirPath + File.separator)) { + throw new TaierDefineException("Invalid kerberos directory"); + } + FileUtils.deleteDirectory(kerberosDir); } diff --git a/taier-data-develop/src/test/java/com/dtstack/taier/develop/controller/console/UploadControllerTest.java b/taier-data-develop/src/test/java/com/dtstack/taier/develop/controller/console/UploadControllerTest.java new file mode 100644 index 0000000000..4485c48651 --- /dev/null +++ b/taier-data-develop/src/test/java/com/dtstack/taier/develop/controller/console/UploadControllerTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.dtstack.taier.develop.controller.console; + +import com.dtstack.taier.common.exception.TaierDefineException; +import com.dtstack.taier.dao.dto.Resource; +import com.google.common.collect.Lists; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.multipart.MultipartFile; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; + +public class UploadControllerTest { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void testGetResourcesFromFilesSaveFileUnderUploadDir() throws Exception { + File uploadDir = temporaryFolder.newFolder("file-uploads"); + ReflectionTestUtils.setField(UploadController.class, "uploadsDir", uploadDir.getAbsolutePath()); + UploadController uploadController = new UploadController(); + MultipartFile multipartFile = new MockMultipartFile("fileName", "config.json", + "application/json", "{\"k\":\"v\"}".getBytes(StandardCharsets.UTF_8)); + + List resources = ReflectionTestUtils.invokeMethod(uploadController, + "getResourcesFromFiles", Lists.newArrayList(multipartFile)); + + Assert.assertNotNull(resources); + Assert.assertEquals(1, resources.size()); + Resource resource = resources.get(0); + Assert.assertEquals("config.json", resource.getFileName()); + Assert.assertEquals("fileName", resource.getKey()); + File savedFile = new File(resource.getUploadedFileName()); + Assert.assertTrue(savedFile.exists()); + Assert.assertEquals(uploadDir.getCanonicalPath(), savedFile.getParentFile().getCanonicalPath()); + Assert.assertEquals("{\"k\":\"v\"}", new String(Files.readAllBytes(savedFile.toPath()), StandardCharsets.UTF_8)); + } + + @Test(expected = TaierDefineException.class) + public void testGetResourcesFromFilesRejectPathTraversalFileName() throws Exception { + File uploadDir = temporaryFolder.newFolder("file-uploads"); + ReflectionTestUtils.setField(UploadController.class, "uploadsDir", uploadDir.getAbsolutePath()); + UploadController uploadController = new UploadController(); + MultipartFile multipartFile = new MockMultipartFile("fileName", "../../../../tmp/x.txt", + "application/octet-stream", "x".getBytes(StandardCharsets.UTF_8)); + + ReflectionTestUtils.invokeMethod(uploadController, + "getResourcesFromFiles", Lists.newArrayList(multipartFile)); + } +} diff --git a/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleClusterServiceTest.java b/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleClusterServiceTest.java new file mode 100644 index 0000000000..12f63c2847 --- /dev/null +++ b/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleClusterServiceTest.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.dtstack.taier.develop.service.console; + +import com.dtstack.taier.common.exception.TaierDefineException; +import com.dtstack.taier.dao.domain.Cluster; +import com.dtstack.taier.dao.mapper.ClusterMapper; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ConsoleClusterServiceTest { + + private ConsoleClusterService consoleClusterService; + + private ClusterMapper clusterMapper; + + @Before + public void setUp() { + consoleClusterService = new ConsoleClusterService(); + clusterMapper = mock(ClusterMapper.class); + ReflectionTestUtils.setField(consoleClusterService, "clusterMapper", clusterMapper); + } + + @Test + public void testAddCluster() { + when(clusterMapper.getByClusterName("cluster_a")).thenReturn(null); + doAnswer(invocation -> { + Cluster cluster = invocation.getArgumentAt(0, Cluster.class); + cluster.setId(1L); + return 1; + }).when(clusterMapper).insert(any(Cluster.class)); + + Long clusterId = consoleClusterService.addCluster("cluster_a"); + + Assert.assertEquals(Long.valueOf(1L), clusterId); + verify(clusterMapper).insert(any(Cluster.class)); + } + + @Test(expected = TaierDefineException.class) + public void testAddClusterRejectPathTraversalName() { + try { + consoleClusterService.addCluster("../../../../tmp/x"); + } finally { + verify(clusterMapper, never()).getByClusterName(any(String.class)); + verify(clusterMapper, never()).insert(any(Cluster.class)); + } + } +} diff --git a/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleComponentServiceTest.java b/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleComponentServiceTest.java new file mode 100644 index 0000000000..4c36f0658a --- /dev/null +++ b/taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleComponentServiceTest.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.dtstack.taier.develop.service.console; + +import com.dtstack.taier.common.enums.EComponentType; +import com.dtstack.taier.common.env.EnvironmentContext; +import com.dtstack.taier.dao.domain.Cluster; +import com.dtstack.taier.dao.mapper.ClusterMapper; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.springframework.test.util.ReflectionTestUtils; + +import java.io.File; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ConsoleComponentServiceTest { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void testGetLocalKerberosPathUseClusterIdInsteadOfClusterName() throws Exception { + File tempDir = temporaryFolder.newFolder("temp"); + Long clusterId = 12L; + Cluster cluster = new Cluster(); + cluster.setId(clusterId); + cluster.setClusterName("../../../../tmp/x"); + + ClusterMapper clusterMapper = mock(ClusterMapper.class); + when(clusterMapper.getOne(clusterId)).thenReturn(cluster); + EnvironmentContext environmentContext = mock(EnvironmentContext.class); + when(environmentContext.getTempDir()).thenReturn(tempDir.getAbsolutePath()); + + ConsoleComponentService consoleComponentService = new ConsoleComponentService(); + ReflectionTestUtils.setField(consoleComponentService, "clusterMapper", clusterMapper); + ReflectionTestUtils.setField(consoleComponentService, "env", environmentContext); + + String localKerberosPath = consoleComponentService.getLocalKerberosPath(clusterId, EComponentType.HDFS.getTypeCode()); + File localKerberosDir = new File(localKerberosPath); + + Assert.assertEquals(new File(tempDir, "CLUSTER_12" + File.separator + "HDFS" + File.separator + "kerberos").getPath(), + localKerberosPath); + Assert.assertTrue(localKerberosDir.getCanonicalPath().startsWith(tempDir.getCanonicalPath() + File.separator)); + Assert.assertFalse(localKerberosPath.contains("..")); + Assert.assertFalse(localKerberosPath.contains("tmp/x")); + } +} diff --git a/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/main/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtil.java b/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/main/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtil.java index 202f62a531..1fc27d952d 100644 --- a/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/main/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtil.java +++ b/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/main/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtil.java @@ -139,6 +139,7 @@ public static List unzipFile(String zipLocation, String targetLocation) { } private static File resolveZipEntryFile(File baseDir, String basePath, String entryName) throws IOException { + validateZipEntryName(entryName); File targetFile = new File(baseDir, entryName); String targetPath = targetFile.getCanonicalPath(); if (!targetPath.equals(basePath) && !targetPath.startsWith(basePath + File.separator)) { @@ -147,6 +148,15 @@ private static File resolveZipEntryFile(File baseDir, String basePath, String en return targetFile; } + private static void validateZipEntryName(String entryName) { + if (entryName == null + || entryName.startsWith("/") + || entryName.startsWith("\\") + || new File(entryName).isAbsolute()) { + throw new SourceException(String.format("Zip entry is outside of target dir: %s", entryName)); + } + } + private static String getCanonicalDirPath(File dir) throws IOException { makeDirs(dir); return dir.getCanonicalPath(); diff --git a/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/test/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtilTest.java b/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/test/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtilTest.java new file mode 100644 index 0000000000..60ecaba629 --- /dev/null +++ b/taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/test/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtilTest.java @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.dtstack.taier.datasource.plugin.common.utils; + +import com.dtstack.taier.datasource.api.exception.SourceException; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.FileOutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; + +public class ZipUtilTest { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void testUnzipFile() throws Exception { + File zipFile = temporaryFolder.newFile("normal.zip"); + writeZip(zipFile, "conf/core-site.xml", "content"); + File targetDir = temporaryFolder.newFolder("normal"); + + List files = ZipUtil.unzipFile(zipFile.getAbsolutePath(), targetDir.getAbsolutePath()); + + Assert.assertEquals(1, files.size()); + File extractedFile = new File(targetDir, "conf/core-site.xml"); + Assert.assertTrue(extractedFile.isFile()); + Assert.assertEquals("content", new String(Files.readAllBytes(extractedFile.toPath()), StandardCharsets.UTF_8)); + } + + @Test + public void testRejectZipSlipEntry() throws Exception { + File zipFile = temporaryFolder.newFile("slip.zip"); + writeZip(zipFile, "../evil.txt", "evil"); + File targetDir = temporaryFolder.newFolder("slip"); + File escapedFile = new File(targetDir.getParentFile(), "evil.txt"); + + try { + ZipUtil.unzipFile(zipFile.getAbsolutePath(), targetDir.getAbsolutePath()); + Assert.fail("Zip Slip entry should be rejected"); + } catch (SourceException e) { + Assert.assertTrue(e.getMessage().contains("outside of target dir")); + } + Assert.assertFalse(escapedFile.exists()); + } + + @Test + public void testRejectAbsolutePathZipEntry() throws Exception { + File targetDir = temporaryFolder.newFolder("absolute"); + File escapedFile = temporaryFolder.newFile("absolute-evil.txt"); + Files.delete(escapedFile.toPath()); + File zipFile = temporaryFolder.newFile("absolute.zip"); + writeZip(zipFile, escapedFile.getAbsolutePath(), "evil"); + + try { + ZipUtil.unzipFile(zipFile.getAbsolutePath(), targetDir.getAbsolutePath()); + Assert.fail("Absolute path zip entry should be rejected"); + } catch (SourceException e) { + Assert.assertTrue(e.getMessage().contains("outside of target dir")); + } + Assert.assertFalse(escapedFile.exists()); + } + + private static void writeZip(File zipFile, String entryName, String content) throws Exception { + try (org.apache.tools.zip.ZipOutputStream zipOutputStream = + new org.apache.tools.zip.ZipOutputStream(new FileOutputStream(zipFile))) { + zipOutputStream.putNextEntry(new org.apache.tools.zip.ZipEntry(entryName)); + zipOutputStream.write(content.getBytes(StandardCharsets.UTF_8)); + zipOutputStream.closeEntry(); + } + } +}