Skip to content

Commit 8009845

Browse files
zhou-haozhou-hao
committed
fix(file): 修复文件上传路径解析漏洞，增强对非法文件名的校验
#344
1 parent 76cb89f commit 8009845

File tree

2 files changed

+123
-21
lines changed

2 files changed

+123
-21
lines changed

‎hsweb-system/hsweb-system-file/src/main/java/org/hswebframework/web/file/FileUploadProperties.java‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,21 @@
44
import lombok.Setter;
55
import org.apache.commons.collections4.CollectionUtils;
66
import org.hswebframework.utils.time.DateFormatter;
7+
import org.hswebframework.web.authorization.exception.AccessDenyException;
78
import org.hswebframework.web.id.IDGenerator;
89
import org.springframework.boot.context.properties.ConfigurationProperties;
910
import org.springframework.http.MediaType;
1011

1112
import java.io.File;
12-
import java.io.IOException;
13+
import java.net.URLDecoder;
14+
import java.nio.charset.StandardCharsets;
1315
import java.nio.file.Files;
16+
import java.nio.file.InvalidPathException;
1417
import java.nio.file.Path;
1518
import java.nio.file.Paths;
1619
import java.nio.file.attribute.PosixFileAttributeView;
1720
import java.nio.file.attribute.PosixFilePermission;
18-
import java.util.Collections;
21+
import java.text.Normalizer;
1922
import java.util.Date;
2023
import java.util.Locale;
2124
import java.util.Set;
@@ -92,28 +95,49 @@ public boolean denied(String name, MediaType mediaType) {
9295
return defaultDeny;
9396
}
9497

98+
public static String resolveExtension(String name) {
99+
int lastIndex = name.lastIndexOf(".");
100+
if (lastIndex < 0) {
101+
return "";
102+
}
103+
return name.substring(lastIndex).toLowerCase(Locale.ROOT);
104+
}
105+
95106
public StaticFileInfo createStaticSavePath(String name) {
96107
String fileName = IDGenerator.SNOW_FLAKE_STRING.generate();
97108
String filePath = DateFormatter.toString(new Date(), "yyyyMMdd");
109+
try {
110+
name = Paths
111+
.get(Normalizer
112+
.normalize(name, Normalizer.Form.NFKC)
113+
.replace("\\", "/"))
114+
.toFile()
115+
.getName();
116+
} catch (InvalidPathException e) {
117+
throw new AccessDenyException.NoStackTrace();
118+
}
98119

99120
//文件后缀
100-
String suffix = name.contains(".") ?
101-
name.substring(name.lastIndexOf(".")) : "";
121+
String suffix = resolveExtension(name);
102122

103123
StaticFileInfo info = new StaticFileInfo();
104124

105-
if (useOriginalFileName) {
125+
// 仅支持 字母数字组成的文件名
126+
if (useOriginalFileName && name.matches("^[a-zA-Z0-9._-]+$")) {
106127
filePath = filePath + "/" + fileName;
107128
fileName = name;
108129
} else {
109130
fileName = fileName + suffix;
110131
}
111132
String absPath = staticFilePath.concat("/").concat(filePath);
112-
new File(absPath).mkdirs();
113133

114-
info.location = staticLocation + "/" + filePath + "/" + fileName;
115-
info.savePath = absPath + "/" + fileName;
134+
boolean ignore = new File(absPath).mkdirs();
135+
136+
Path fullPath = Paths.get(absPath, fileName);
137+
info.savePath = fullPath.normalize().toString();
138+
116139
info.relativeLocation = filePath + "/" + fileName;
140+
info.location = staticLocation + "/" + filePath + "/" + fileName;
117141
return info;
118142
}
119143

‎hsweb-system/hsweb-system-file/src/test/java/org/hswebframework/web/file/FileUploadPropertiesTest.java‎

Lines changed: 91 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package org.hswebframework.web.file;
22

3+
import org.hswebframework.web.authorization.exception.AccessDenyException;
34
import org.junit.Test;
45
import org.springframework.http.MediaType;
56

7+
import java.text.Normalizer;
68
import java.util.Arrays;
79
import java.util.HashSet;
810

@@ -12,17 +14,17 @@ public class FileUploadPropertiesTest {
1214

1315

1416
@Test
15-
public void testNoSet(){
16-
FileUploadProperties uploadProperties=new FileUploadProperties();
17+
public void testNoSet() {
18+
FileUploadProperties uploadProperties = new FileUploadProperties();
1719
assertFalse(uploadProperties.denied("test.xls", MediaType.ALL));
1820

1921
assertFalse(uploadProperties.denied("test.exe", MediaType.ALL));
2022
}
2123

2224
@Test
23-
public void testDenyWithAllow(){
24-
FileUploadProperties uploadProperties=new FileUploadProperties();
25-
uploadProperties.setAllowFiles(new HashSet<>(Arrays.asList("xls","json")));
25+
public void testDenyWithAllow() {
26+
FileUploadProperties uploadProperties = new FileUploadProperties();
27+
uploadProperties.setAllowFiles(new HashSet<>(Arrays.asList("xls", "json")));
2628

2729
assertFalse(uploadProperties.denied("test.xls", MediaType.ALL));
2830
assertFalse(uploadProperties.denied("test.XLS", MediaType.ALL));
@@ -31,30 +33,30 @@ public void testDenyWithAllow(){
3133
}
3234

3335
@Test
34-
public void testDenyWithAllowMediaType(){
35-
FileUploadProperties uploadProperties=new FileUploadProperties();
36-
uploadProperties.setAllowMediaType(new HashSet<>(Arrays.asList("application/xls","application/json")));
36+
public void testDenyWithAllowMediaType() {
37+
FileUploadProperties uploadProperties = new FileUploadProperties();
38+
uploadProperties.setAllowMediaType(new HashSet<>(Arrays.asList("application/xls", "application/json")));
3739

3840
assertFalse(uploadProperties.denied("test.json", MediaType.APPLICATION_JSON));
3941

4042
assertTrue(uploadProperties.denied("test.exe", MediaType.ALL));
4143
}
4244

4345

44-
4546
@Test
46-
public void testDenyWithDenyMediaType(){
47-
FileUploadProperties uploadProperties=new FileUploadProperties();
47+
public void testDenyWithDenyMediaType() {
48+
FileUploadProperties uploadProperties = new FileUploadProperties();
4849
uploadProperties.setDenyMediaType(new HashSet<>(Arrays.asList("application/json")));
4950

5051
assertFalse(uploadProperties.denied("test.xls", MediaType.ALL));
5152

5253
assertTrue(uploadProperties.denied("test.exe", MediaType.APPLICATION_JSON));
5354

5455
}
56+
5557
@Test
56-
public void testDenyWithDeny(){
57-
FileUploadProperties uploadProperties=new FileUploadProperties();
58+
public void testDenyWithDeny() {
59+
FileUploadProperties uploadProperties = new FileUploadProperties();
5860
uploadProperties.setDenyFiles(new HashSet<>(Arrays.asList("exe")));
5961

6062
assertFalse(uploadProperties.denied("test.xls", MediaType.ALL));
@@ -64,4 +66,80 @@ public void testDenyWithDeny(){
6466
}
6567

6668

69+
@Test
70+
// https://github.com/hs-web/hsweb-framework/issues/344
71+
public void testIllegalFileName() {
72+
FileUploadProperties uploadProperties = new FileUploadProperties();
73+
uploadProperties.setUseOriginalFileName(true);
74+
75+
// 基本的路径遍历攻击
76+
FileUploadProperties.StaticFileInfo fileInfo = uploadProperties
77+
.createStaticSavePath("../../../../pom.xml");
78+
assertFalse(fileInfo.getSavePath().contains("../"));
79+
assertFalse(fileInfo.getRelativeLocation().contains("../"));
80+
assertFalse(fileInfo.getLocation().contains("../"));
81+
82+
// Windows风格的路径遍历攻击
83+
fileInfo = uploadProperties.createStaticSavePath("..\\..\\..\\..\\pom.xml");
84+
assertFalse(fileInfo.getSavePath().contains("..\\"));
85+
assertFalse(fileInfo.getRelativeLocation().contains("..\\"));
86+
assertFalse(fileInfo.getLocation().contains("..\\"));
87+
88+
// URL编码的路径遍历
89+
fileInfo = uploadProperties.createStaticSavePath("..%2F..%2F..%2F..%2Fpom.xml");
90+
assertFalse(fileInfo.getSavePath().contains("../"));
91+
assertFalse(fileInfo.getSavePath().contains("..%2F"));
92+
assertFalse(fileInfo.getRelativeLocation().contains("../"));
93+
assertFalse(fileInfo.getLocation().contains("../"));
94+
95+
// 双重URL编码
96+
fileInfo = uploadProperties.createStaticSavePath("..%252F..%252F..%252Fpom.xml");
97+
assertFalse(fileInfo.getSavePath().contains("../"));
98+
assertFalse(fileInfo.getSavePath().contains("..%2F"));
99+
assertFalse(fileInfo.getSavePath().contains("..%252F"));
100+
101+
// Unicode编码的路径遍历
102+
fileInfo = uploadProperties.createStaticSavePath("..%c0%af..%c0%afpom.xml");
103+
assertFalse(fileInfo.getSavePath().contains("../"));
104+
assertFalse(fileInfo.getRelativeLocation().contains("../"));
105+
106+
// 绝对路径攻击 - Linux
107+
fileInfo = uploadProperties.createStaticSavePath("/etc/passwd");
108+
assertFalse(fileInfo.getSavePath().startsWith("/etc/"));
109+
assertFalse(fileInfo.getLocation().contains("/etc/passwd"));
110+
111+
// 绝对路径攻击 - Windows
112+
fileInfo = uploadProperties.createStaticSavePath("C:\\Windows\\System32\\config\\sam");
113+
assertFalse(fileInfo.getSavePath().contains("C:\\"));
114+
assertFalse(fileInfo.getSavePath().contains("System32"));
115+
116+
// 混合斜杠
117+
fileInfo = uploadProperties.createStaticSavePath("..\\../..\\../pom.xml");
118+
assertFalse(fileInfo.getSavePath().contains("../"));
119+
assertFalse(fileInfo.getSavePath().contains("..\\"));
120+
121+
// 过度的路径遍历
122+
fileInfo = uploadProperties.createStaticSavePath("../../../../../../../../../../../../etc/passwd");
123+
assertFalse(fileInfo.getSavePath().contains("../"));
124+
assertFalse(fileInfo.getLocation().contains("/etc/"));
125+
126+
127+
// // 带有空字节注入
128+
assertThrows(AccessDenyException.class,
129+
()->{
130+
uploadProperties.createStaticSavePath("../../pom.xml\0.jpg");
131+
});
132+
133+
// 点和斜杠的各种组合
134+
fileInfo = uploadProperties.createStaticSavePath("....//....//pom.xml");
135+
assertFalse(fileInfo.getSavePath().contains(".."));
136+
assertFalse(fileInfo.getSavePath().contains("//"));
137+
138+
// 反斜杠编码
139+
fileInfo = uploadProperties.createStaticSavePath("..%5c..%5cpom.xml");
140+
assertFalse(fileInfo.getSavePath().contains("..\\"));
141+
assertFalse(fileInfo.getSavePath().contains("..%5c"));
142+
}
143+
144+
67145
}

0 commit comments

Comments
 (0)