Skip to content

Commit 916d2a6

Browse files
authored
BugFix:fix Code QL alerts (#664)
<!-- Please provide brief information about the PR, what it contains & its purpose, new behaviors after the change. And let us know here if you need any help: https://github.com/microsoft/HydraLab/issues/new --> ## Description <!-- A few words to explain your changes --> ### Linked GitHub issue ID: # ## Pull Request Checklist <!-- Put an x in the boxes that apply. This is simply a reminder of what we are going to look for before merging your code. --> - [ ] Tests for the changes have been added (for bug fixes / features) - [x] Code compiles correctly with all tests are passed. - [ ] I've read the [contributing guide](https://github.com/microsoft/HydraLab/blob/main/CONTRIBUTING.md#making-changes-to-the-code) and followed the recommended practices. - [ ] [Wikis](https://github.com/microsoft/HydraLab/wiki) or [README](https://github.com/microsoft/HydraLab/blob/main/README.md) have been reviewed and added / updated if needed (for bug fixes / features) ### Does this introduce a breaking change? *If this introduces a breaking change for Hydra Lab users, please describe the impact and migration path.* - [ ] Yes - [x] No ## How you tested it *Please make sure the change is tested, you can test it by adding UTs, do local test and share the screenshots, etc.* ![image](https://github.com/user-attachments/assets/15a37ed5-a0c9-4290-84df-6628f26f7128) Please check the type of change your PR introduces: - [x] Bugfix - [ ] Feature - [ ] Technical design - [ ] Build related changes - [ ] Refactoring (no functional changes, no api changes) - [ ] Code style update (formatting, renaming) or Documentation content changes - [ ] Other (please describe): ### Feature UI screenshots or Technical design diagrams *If this is a relatively large or complex change, kick it off by drawing the tech design with PlantUML and explaining why you chose the solution you did and what alternatives you considered, etc...*
1 parent cafabb5 commit 916d2a6

File tree

9 files changed

+62
-21
lines changed

9 files changed

+62
-21
lines changed

center/src/main/java/com/microsoft/hydralab/center/controller/AuthController.java

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@
1313
import com.microsoft.hydralab.common.util.Const;
1414
import com.microsoft.hydralab.common.util.LogUtils;
1515
import org.apache.commons.io.FileUtils;
16+
import org.apache.commons.io.IOUtils;
1617
import org.apache.commons.lang3.StringUtils;
18+
import org.springframework.http.CacheControl;
19+
import org.springframework.http.HttpHeaders;
1720
import org.springframework.http.HttpStatus;
1821
import org.springframework.http.MediaType;
22+
import org.springframework.http.ResponseEntity;
1923
import org.springframework.security.access.prepost.PreAuthorize;
2024
import org.springframework.security.core.annotation.CurrentSecurityContext;
2125
import org.springframework.web.bind.annotation.GetMapping;
@@ -31,7 +35,6 @@
3135
import javax.servlet.http.HttpServletResponse;
3236
import java.io.IOException;
3337
import java.io.InputStream;
34-
import java.io.OutputStream;
3538
import java.util.List;
3639

3740
@RestController
@@ -177,24 +180,27 @@ public Result getUserInfo(@CurrentSecurityContext SysUser requestor) {
177180
*/
178181
@GetMapping(value = {"/api/auth/getUserPhoto"}, produces = MediaType.IMAGE_JPEG_VALUE)
179182
@ResponseBody
180-
public void getUserPhoto(@CurrentSecurityContext SysUser requestor,
181-
HttpServletResponse response) {
182-
try {
183-
InputStream inputStream = null;
184-
if (requestor == null || requestor.getAccessToken() == null) {
185-
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
186-
} else {
183+
public ResponseEntity<byte[]> getUserPhoto(@CurrentSecurityContext SysUser requestor,
184+
HttpServletResponse response) {
185+
HttpHeaders headers = new HttpHeaders();
186+
InputStream inputStream = null;
187+
if (requestor == null || requestor.getAccessToken() == null) {
188+
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
189+
} else {
190+
try {
187191
inputStream = authUtil.requestPhoto(requestor.getAccessToken());
192+
} catch (Exception e) {
193+
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
188194
}
189-
byte[] bytes = new byte[inputStream.available()];
190-
inputStream.read(bytes, 0, inputStream.available());
191-
response.setContentType(MediaType.IMAGE_JPEG_VALUE);
192-
OutputStream out = response.getOutputStream();
193-
out.write(bytes);
194-
out.flush();
195-
out.close();
196-
} catch (Exception e) {
197-
e.printStackTrace();
198195
}
196+
byte[] bytes = new byte[0];
197+
try {
198+
bytes = IOUtils.toByteArray(inputStream);
199+
} catch (IOException e) {
200+
throw new RuntimeException(e);
201+
}
202+
headers.setCacheControl(CacheControl.noCache().getHeaderValue());
203+
ResponseEntity<byte[]> responseEntity = new ResponseEntity<>(bytes, headers, HttpStatus.OK);
204+
return responseEntity;
199205
}
200206
}

center/src/main/java/com/microsoft/hydralab/center/controller/PackageSetController.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,11 @@ public Result addAttachment(@CurrentSecurityContext SysUser requestor,
415415
return Result.error(HttpStatus.BAD_REQUEST.value(), "Error fileType");
416416
}
417417
try {
418-
String newFileName = FileUtil.getLegalFileName(attachment.getOriginalFilename());
418+
String originalFilename = attachment.getOriginalFilename();
419+
if (originalFilename.contains("..") || originalFilename.contains("/") || originalFilename.contains("\\")) {
420+
throw new HydraLabRuntimeException("Invalid filename");
421+
}
422+
String newFileName = FileUtil.getLegalFileName(originalFilename);
419423
String fileRelativeParent = FileUtil.getPathForToday();
420424
String parentDir = CENTER_FILE_BASE_DIR + fileRelativeParent;
421425

center/src/main/java/com/microsoft/hydralab/center/controller/StorageController.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.io.File;
3030
import java.io.IOException;
3131
import java.io.InputStream;
32+
import java.nio.file.Path;
33+
import java.nio.file.Paths;
3234

3335
/**
3436
* @author Li Shen
@@ -93,6 +95,12 @@ public void postDownloadFile(HttpServletRequest request,
9395
throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path, file name should not include ';'!");
9496
}
9597

98+
Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath();
99+
Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath();
100+
if (!filePath.startsWith(publicFolder + File.separator)) {
101+
throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path");
102+
}
103+
96104
File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri);
97105
if (!file.exists()) {
98106
throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), String.format("File %s not exist!", fileUri));

center/src/main/java/com/microsoft/hydralab/center/controller/TestDetailController.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,9 @@ public Result getSmartTestGraphPhoto(@CurrentSecurityContext SysUser requestor,
318318
}
319319
try {
320320
File graphZipFile = loadGraphFile(fileId);
321-
321+
if (node.contains("..") || node.contains("/") || node.contains("\\")) {
322+
throw new HydraLabRuntimeException("Invalid node name");
323+
}
322324
File nodeFile = new File(graphZipFile.getParentFile().getAbsolutePath(), node + "/" + node + "-0.jpg");
323325
if (!nodeFile.exists()) {
324326
throw new HydraLabRuntimeException("Graph photo file not found");

center/src/main/java/com/microsoft/hydralab/center/service/DeviceAgentManagementService.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
import java.io.File;
5757
import java.io.IOException;
5858
import java.nio.ByteBuffer;
59+
import java.nio.file.Path;
60+
import java.nio.file.Paths;
5961
import java.util.ArrayList;
6062
import java.util.Collections;
6163
import java.util.Date;
@@ -777,6 +779,12 @@ private JSONObject runT2CTest(TestTaskSpec testTaskSpec) {
777779
Map<String, Integer> aggregateDeviceCountMap = new HashMap<>();
778780
for (StorageFileInfo testJsonInfo : attachmentTestJsonList) {
779781
Map<String, Integer> deviceCountMap = new HashMap<>();
782+
783+
Path publicFolder = Paths.get(CENTER_FILE_BASE_DIR).normalize().toAbsolutePath();
784+
Path filePath = publicFolder.resolve(testJsonInfo.getBlobPath()).normalize().toAbsolutePath();
785+
if (!filePath.startsWith(publicFolder + File.separator)) {
786+
throw new HydraLabRuntimeException("Invalid blob path");
787+
}
780788
File testJsonFile = new File(CENTER_FILE_BASE_DIR, testJsonInfo.getBlobPath());
781789
TestInfo testInfo;
782790
try {

center/src/main/java/com/microsoft/hydralab/center/util/AuthUtil.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package com.microsoft.hydralab.center.util;
55

66
import com.alibaba.fastjson.JSONObject;
7+
import com.alibaba.fastjson.parser.ParserConfig;
78
import com.microsoft.hydralab.common.util.RestTemplateConfig;
89
import com.microsoft.hydralab.common.util.FileUtil;
910
import org.apache.commons.codec.binary.Base64;
@@ -95,6 +96,7 @@ public JSONObject decodeAccessToken(String accessToken) {
9596
String[] pieces = accessToken.split("\\.");
9697
String b64payload = pieces[1];
9798
String jsonString = new String(Base64.decodeBase64(b64payload), FileUtil.UTF_8);
99+
ParserConfig.getGlobalInstance().setSafeMode(true);
98100
userInfo = JSONObject.parseObject(jsonString);
99101
} catch (Exception e) {
100102
e.printStackTrace();

center/src/main/java/com/microsoft/hydralab/center/util/LocalStorageIOUtil.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import java.io.IOException;
1717
import java.io.InputStream;
1818
import java.io.OutputStream;
19+
import java.nio.file.Path;
20+
import java.nio.file.Paths;
1921

2022
/**
2123
* @author Li Shen
@@ -28,6 +30,11 @@ private LocalStorageIOUtil() {
2830
}
2931

3032
public static void copyUploadedStreamToFile(InputStream inputStream, String fileUri) {
33+
Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath();
34+
Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath();
35+
if (!filePath.startsWith(publicFolder + File.separator)) {
36+
throw new HydraLabRuntimeException("Invalid file uri");
37+
}
3138
File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri);
3239
File parentDirFile = new File(file.getParent());
3340
if (!parentDirFile.exists() && !parentDirFile.mkdirs()) {

common/src/main/java/com/microsoft/hydralab/common/util/AttachmentService.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,11 @@ public File verifyAndSaveFile(@NotNull MultipartFile originFile, String parentDi
253253
if (!parentDirFile.exists() && !parentDirFile.mkdirs()) {
254254
throw new HydraLabRuntimeException(HttpStatus.INTERNAL_SERVER_ERROR.value(), "mkdirs failed!");
255255
}
256-
String filename = FileUtil.getLegalFileName(originFile.getOriginalFilename());
256+
String originalFilename = originFile.getOriginalFilename();
257+
if (originalFilename.contains("..") || originalFilename.contains("/") || originalFilename.contains("\\")) {
258+
throw new HydraLabRuntimeException("Invalid filename");
259+
}
260+
String filename = FileUtil.getLegalFileName(originalFilename);
257261
String fileSuffix = null;
258262
boolean isMatch = false;
259263
if (filename == null) {

hercules/util.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def create_file(path, content):
2323

2424
def filter_functions(class_name, functions):
2525
ret = functions
26-
ret = [s for s in ret if re.match(r"public\s+\w+\s+\w+\((\w+,?\s*)*\)", s)]
26+
ret = [s for s in ret if re.match(r"public\s+\w+\s+\w+\((\w+,\s*|\w+\s*)*\)", s)]
2727
ret = [s for s in ret if (class_name + "(") not in s]
2828
return ret
2929

0 commit comments

Comments
 (0)