Skip to content

Commit ec8c59c

Browse files
zcswl7961zcswl7961
authored
[feat_1204][taier-data-develop] fix path traversal #1204 
1 parent 572773c commit ec8c59c

9 files changed

Lines changed: 387 additions & 2 deletions

File tree

‎taier-common/src/main/java/com/dtstack/taier/common/util/ZipUtil.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ private static List<File> upzipFile(File zipFile, String descDir, UnzipContext c
301301
}
302302

303303
private static File resolveZipEntryFile(File baseDir, String basePath, String entryName) throws IOException {
304+
validateZipEntryName(entryName);
304305
File targetFile = new File(baseDir, entryName);
305306
String targetPath = targetFile.getCanonicalPath();
306307
if (!targetPath.equals(basePath) && !targetPath.startsWith(basePath + File.separator)) {
@@ -309,6 +310,15 @@ private static File resolveZipEntryFile(File baseDir, String basePath, String en
309310
return targetFile;
310311
}
311312

313+
private static void validateZipEntryName(String entryName) throws IOException {
314+
if (entryName == null
315+
|| entryName.startsWith("/")
316+
|| entryName.startsWith("\\")
317+
|| new File(entryName).isAbsolute()) {
318+
throw new IOException(String.format("zip entry is outside of target dir: %s", entryName));
319+
}
320+
}
321+
312322
private static String getCanonicalDirPath(File dir) throws IOException {
313323
makeDirs(dir);
314324
return dir.getCanonicalPath();

‎taier-common/src/test/java/com/dtstack/taier/common/util/ZipUtilTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,23 @@ public void testRejectZipSlipEntry() throws Exception {
6868
Assert.assertFalse(escapedFile.exists());
6969
}
7070

71+
@Test
72+
public void testRejectAbsolutePathZipEntry() throws Exception {
73+
File targetDir = temporaryFolder.newFolder("absolute");
74+
File escapedFile = temporaryFolder.newFile("absolute-evil.txt");
75+
Files.delete(escapedFile.toPath());
76+
File zipFile = temporaryFolder.newFile("absolute.zip");
77+
writeApacheZip(zipFile, escapedFile.getAbsolutePath(), "evil");
78+
79+
try {
80+
ZipUtil.upzipFile(zipFile, targetDir.getAbsolutePath());
81+
Assert.fail("Absolute path zip entry should be rejected");
82+
} catch (TaierDefineException e) {
83+
Assert.assertTrue(e.getMessage().contains("outside of target dir"));
84+
}
85+
Assert.assertFalse(escapedFile.exists());
86+
}
87+
7188
@Test
7289
public void testRejectTooManyEntries() throws Exception {
7390
File zipFile = temporaryFolder.newFile("too-many.zip");
@@ -129,6 +146,15 @@ private static void writeZip(File zipFile, ZipItem... items) throws IOException
129146
}
130147
}
131148

149+
private static void writeApacheZip(File zipFile, String entryName, String content) throws IOException {
150+
try (org.apache.tools.zip.ZipOutputStream zipOutputStream =
151+
new org.apache.tools.zip.ZipOutputStream(new FileOutputStream(zipFile))) {
152+
zipOutputStream.putNextEntry(new org.apache.tools.zip.ZipEntry(entryName));
153+
zipOutputStream.write(content.getBytes(StandardCharsets.UTF_8));
154+
zipOutputStream.closeEntry();
155+
}
156+
}
157+
132158
private static class ZipItem {
133159
private final String name;
134160
private final String content;

‎taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleClusterService.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
import com.dtstack.taier.scheduler.service.ComponentService;
3737
import com.dtstack.taier.scheduler.vo.ComponentVO;
3838
import org.apache.commons.collections.CollectionUtils;
39+
import org.apache.commons.lang3.StringUtils;
3940
import org.springframework.beans.factory.annotation.Autowired;
4041
import org.springframework.stereotype.Component;
4142

43+
import java.io.File;
4244
import java.util.ArrayList;
4345
import java.util.Comparator;
4446
import java.util.HashMap;
@@ -65,6 +67,7 @@ public class ConsoleClusterService {
6567
private ComponentService componentService;
6668

6769
public Long addCluster(String clusterName) {
70+
checkClusterName(clusterName);
6871
if (clusterMapper.getByClusterName(clusterName) != null) {
6972
throw new TaierDefineException(ErrorCode.NAME_ALREADY_EXIST.getDescription());
7073
}
@@ -74,6 +77,17 @@ public Long addCluster(String clusterName) {
7477
return cluster.getId();
7578
}
7679

80+
private void checkClusterName(String clusterName) {
81+
if (StringUtils.isBlank(clusterName)
82+
|| clusterName.contains("/")
83+
|| clusterName.contains("\\")
84+
|| clusterName.contains("..")
85+
|| clusterName.indexOf('\0') >= 0
86+
|| new File(clusterName).isAbsolute()) {
87+
throw new TaierDefineException("Invalid cluster name");
88+
}
89+
}
90+
7791
public IPage<Cluster> pageQuery(int currentPage, int pageSize) {
7892
Page<Cluster> page = new Page<>(currentPage, pageSize);
7993
return clusterMapper.selectPage(page, Wrappers.lambdaQuery(Cluster.class).eq(

‎taier-data-develop/src/main/java/com/dtstack/taier/develop/service/console/ConsoleComponentService.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ public class ConsoleComponentService {
112112

113113
private static final Logger LOGGER = LoggerFactory.getLogger(ComponentService.class);
114114

115+
private static final String LOCAL_KERBEROS_CLUSTER_DIR_PREFIX = "CLUSTER_";
116+
115117
@Autowired
116118
private ComponentMapper componentMapper;
117119

@@ -491,7 +493,7 @@ private String updateComponentKerberosFile(Long clusterId, Component addComponen
491493
//删除本地文件夹
492494
String kerberosPath = this.getLocalKerberosPath(clusterId, addComponent.getComponentTypeCode());
493495
try {
494-
FileUtils.deleteDirectory(new File(kerberosPath));
496+
deleteLocalKerberosDirectory(kerberosPath);
495497
} catch (IOException e) {
496498
LOGGER.error("delete old kerberos directory {} error", kerberosPath, e);
497499
}
@@ -614,7 +616,22 @@ public String getLocalKerberosPath(Long clusterId, Integer componentCode) {
614616
if (null == one) {
615617
throw new TaierDefineException(ErrorCode.CANT_NOT_FIND_CLUSTER);
616618
}
617-
return env.getTempDir() + File.separator + one.getClusterName() + File.separator + EComponentType.getByCode(componentCode).name() + File.separator + KERBEROS;
619+
if (StringUtils.isBlank(env.getTempDir())) {
620+
throw new TaierDefineException("Temp dir cannot be empty");
621+
}
622+
return env.getTempDir() + File.separator + LOCAL_KERBEROS_CLUSTER_DIR_PREFIX + clusterId
623+
+ File.separator + EComponentType.getByCode(componentCode).name() + File.separator + KERBEROS;
624+
}
625+
626+
private void deleteLocalKerberosDirectory(String kerberosPath) throws IOException {
627+
File tempDir = new File(env.getTempDir()).getCanonicalFile();
628+
File kerberosDir = new File(kerberosPath).getCanonicalFile();
629+
String tempDirPath = tempDir.getPath();
630+
String kerberosDirPath = kerberosDir.getPath();
631+
if (!kerberosDirPath.startsWith(tempDirPath + File.separator)) {
632+
throw new TaierDefineException("Invalid kerberos directory");
633+
}
634+
FileUtils.deleteDirectory(kerberosDir);
618635
}
619636

620637

‎taier-data-develop/src/test/java/com/dtstack/taier/develop/controller/console/UploadControllerTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package com.dtstack.taier.develop.controller.console;
20+
21+
import com.dtstack.taier.common.exception.TaierDefineException;
22+
import com.dtstack.taier.dao.dto.Resource;
23+
import com.google.common.collect.Lists;
24+
import org.junit.Assert;
25+
import org.junit.Rule;
26+
import org.junit.Test;
27+
import org.junit.rules.TemporaryFolder;
28+
import org.springframework.mock.web.MockMultipartFile;
29+
import org.springframework.test.util.ReflectionTestUtils;
30+
import org.springframework.web.multipart.MultipartFile;
31+
32+
import java.io.File;
33+
import java.nio.charset.StandardCharsets;
34+
import java.nio.file.Files;
35+
import java.util.List;
36+
37+
public class UploadControllerTest {
38+
39+
@Rule
40+
public TemporaryFolder temporaryFolder = new TemporaryFolder();
41+
42+
@Test
43+
public void testGetResourcesFromFilesSaveFileUnderUploadDir() throws Exception {
44+
File uploadDir = temporaryFolder.newFolder("file-uploads");
45+
ReflectionTestUtils.setField(UploadController.class, "uploadsDir", uploadDir.getAbsolutePath());
46+
UploadController uploadController = new UploadController();
47+
MultipartFile multipartFile = new MockMultipartFile("fileName", "config.json",
48+
"application/json", "{\"k\":\"v\"}".getBytes(StandardCharsets.UTF_8));
49+
50+
List<Resource> resources = ReflectionTestUtils.invokeMethod(uploadController,
51+
"getResourcesFromFiles", Lists.newArrayList(multipartFile));
52+
53+
Assert.assertNotNull(resources);
54+
Assert.assertEquals(1, resources.size());
55+
Resource resource = resources.get(0);
56+
Assert.assertEquals("config.json", resource.getFileName());
57+
Assert.assertEquals("fileName", resource.getKey());
58+
File savedFile = new File(resource.getUploadedFileName());
59+
Assert.assertTrue(savedFile.exists());
60+
Assert.assertEquals(uploadDir.getCanonicalPath(), savedFile.getParentFile().getCanonicalPath());
61+
Assert.assertEquals("{\"k\":\"v\"}", new String(Files.readAllBytes(savedFile.toPath()), StandardCharsets.UTF_8));
62+
}
63+
64+
@Test(expected = TaierDefineException.class)
65+
public void testGetResourcesFromFilesRejectPathTraversalFileName() throws Exception {
66+
File uploadDir = temporaryFolder.newFolder("file-uploads");
67+
ReflectionTestUtils.setField(UploadController.class, "uploadsDir", uploadDir.getAbsolutePath());
68+
UploadController uploadController = new UploadController();
69+
MultipartFile multipartFile = new MockMultipartFile("fileName", "../../../../tmp/x.txt",
70+
"application/octet-stream", "x".getBytes(StandardCharsets.UTF_8));
71+
72+
ReflectionTestUtils.invokeMethod(uploadController,
73+
"getResourcesFromFiles", Lists.newArrayList(multipartFile));
74+
}
75+
}

‎taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleClusterServiceTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package com.dtstack.taier.develop.service.console;
20+
21+
import com.dtstack.taier.common.exception.TaierDefineException;
22+
import com.dtstack.taier.dao.domain.Cluster;
23+
import com.dtstack.taier.dao.mapper.ClusterMapper;
24+
import org.junit.Assert;
25+
import org.junit.Before;
26+
import org.junit.Test;
27+
import org.springframework.test.util.ReflectionTestUtils;
28+
29+
import static org.mockito.Matchers.any;
30+
import static org.mockito.Mockito.doAnswer;
31+
import static org.mockito.Mockito.mock;
32+
import static org.mockito.Mockito.never;
33+
import static org.mockito.Mockito.verify;
34+
import static org.mockito.Mockito.when;
35+
36+
public class ConsoleClusterServiceTest {
37+
38+
private ConsoleClusterService consoleClusterService;
39+
40+
private ClusterMapper clusterMapper;
41+
42+
@Before
43+
public void setUp() {
44+
consoleClusterService = new ConsoleClusterService();
45+
clusterMapper = mock(ClusterMapper.class);
46+
ReflectionTestUtils.setField(consoleClusterService, "clusterMapper", clusterMapper);
47+
}
48+
49+
@Test
50+
public void testAddCluster() {
51+
when(clusterMapper.getByClusterName("cluster_a")).thenReturn(null);
52+
doAnswer(invocation -> {
53+
Cluster cluster = invocation.getArgumentAt(0, Cluster.class);
54+
cluster.setId(1L);
55+
return 1;
56+
}).when(clusterMapper).insert(any(Cluster.class));
57+
58+
Long clusterId = consoleClusterService.addCluster("cluster_a");
59+
60+
Assert.assertEquals(Long.valueOf(1L), clusterId);
61+
verify(clusterMapper).insert(any(Cluster.class));
62+
}
63+
64+
@Test(expected = TaierDefineException.class)
65+
public void testAddClusterRejectPathTraversalName() {
66+
try {
67+
consoleClusterService.addCluster("../../../../tmp/x");
68+
} finally {
69+
verify(clusterMapper, never()).getByClusterName(any(String.class));
70+
verify(clusterMapper, never()).insert(any(Cluster.class));
71+
}
72+
}
73+
}

‎taier-data-develop/src/test/java/com/dtstack/taier/develop/service/console/ConsoleComponentServiceTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package com.dtstack.taier.develop.service.console;
20+
21+
import com.dtstack.taier.common.enums.EComponentType;
22+
import com.dtstack.taier.common.env.EnvironmentContext;
23+
import com.dtstack.taier.dao.domain.Cluster;
24+
import com.dtstack.taier.dao.mapper.ClusterMapper;
25+
import org.junit.Assert;
26+
import org.junit.Rule;
27+
import org.junit.Test;
28+
import org.junit.rules.TemporaryFolder;
29+
import org.springframework.test.util.ReflectionTestUtils;
30+
31+
import java.io.File;
32+
33+
import static org.mockito.Mockito.mock;
34+
import static org.mockito.Mockito.when;
35+
36+
public class ConsoleComponentServiceTest {
37+
38+
@Rule
39+
public TemporaryFolder temporaryFolder = new TemporaryFolder();
40+
41+
@Test
42+
public void testGetLocalKerberosPathUseClusterIdInsteadOfClusterName() throws Exception {
43+
File tempDir = temporaryFolder.newFolder("temp");
44+
Long clusterId = 12L;
45+
Cluster cluster = new Cluster();
46+
cluster.setId(clusterId);
47+
cluster.setClusterName("../../../../tmp/x");
48+
49+
ClusterMapper clusterMapper = mock(ClusterMapper.class);
50+
when(clusterMapper.getOne(clusterId)).thenReturn(cluster);
51+
EnvironmentContext environmentContext = mock(EnvironmentContext.class);
52+
when(environmentContext.getTempDir()).thenReturn(tempDir.getAbsolutePath());
53+
54+
ConsoleComponentService consoleComponentService = new ConsoleComponentService();
55+
ReflectionTestUtils.setField(consoleComponentService, "clusterMapper", clusterMapper);
56+
ReflectionTestUtils.setField(consoleComponentService, "env", environmentContext);
57+
58+
String localKerberosPath = consoleComponentService.getLocalKerberosPath(clusterId, EComponentType.HDFS.getTypeCode());
59+
File localKerberosDir = new File(localKerberosPath);
60+
61+
Assert.assertEquals(new File(tempDir, "CLUSTER_12" + File.separator + "HDFS" + File.separator + "kerberos").getPath(),
62+
localKerberosPath);
63+
Assert.assertTrue(localKerberosDir.getCanonicalPath().startsWith(tempDir.getCanonicalPath() + File.separator));
64+
Assert.assertFalse(localKerberosPath.contains(".."));
65+
Assert.assertFalse(localKerberosPath.contains("tmp/x"));
66+
}
67+
}

‎taier-datasource/taier-datasource-plugin/taier-datasource-plugin-common/src/main/java/com/dtstack/taier/datasource/plugin/common/utils/ZipUtil.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ public static List<File> unzipFile(String zipLocation, String targetLocation) {
139139
}
140140

141141
private static File resolveZipEntryFile(File baseDir, String basePath, String entryName) throws IOException {
142+
validateZipEntryName(entryName);
142143
File targetFile = new File(baseDir, entryName);
143144
String targetPath = targetFile.getCanonicalPath();
144145
if (!targetPath.equals(basePath) && !targetPath.startsWith(basePath + File.separator)) {
@@ -147,6 +148,15 @@ private static File resolveZipEntryFile(File baseDir, String basePath, String en
147148
return targetFile;
148149
}
149150

151+
private static void validateZipEntryName(String entryName) {
152+
if (entryName == null
153+
|| entryName.startsWith("/")
154+
|| entryName.startsWith("\\")
155+
|| new File(entryName).isAbsolute()) {
156+
throw new SourceException(String.format("Zip entry is outside of target dir: %s", entryName));
157+
}
158+
}
159+
150160
private static String getCanonicalDirPath(File dir) throws IOException {
151161
makeDirs(dir);
152162
return dir.getCanonicalPath();

0 commit comments

Comments
 (0)