Skip to content

Commit

Permalink
BugFix:fix Code QL alerts (#664)
Browse files Browse the repository at this point in the history
<!-- 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...*
  • Loading branch information
zhou9584 authored Dec 2, 2024
1 parent cafabb5 commit 916d2a6
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
import com.microsoft.hydralab.common.util.Const;
import com.microsoft.hydralab.common.util.LogUtils;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.springframework.http.CacheControl;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.annotation.CurrentSecurityContext;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -31,7 +35,6 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.List;

@RestController
Expand Down Expand Up @@ -177,24 +180,27 @@ public Result getUserInfo(@CurrentSecurityContext SysUser requestor) {
*/
@GetMapping(value = {"/api/auth/getUserPhoto"}, produces = MediaType.IMAGE_JPEG_VALUE)
@ResponseBody
public void getUserPhoto(@CurrentSecurityContext SysUser requestor,
HttpServletResponse response) {
try {
InputStream inputStream = null;
if (requestor == null || requestor.getAccessToken() == null) {
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
} else {
public ResponseEntity<byte[]> getUserPhoto(@CurrentSecurityContext SysUser requestor,
HttpServletResponse response) {
HttpHeaders headers = new HttpHeaders();
InputStream inputStream = null;
if (requestor == null || requestor.getAccessToken() == null) {
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
} else {
try {
inputStream = authUtil.requestPhoto(requestor.getAccessToken());
} catch (Exception e) {
inputStream = FileUtils.class.getClassLoader().getResourceAsStream(Const.Path.DEFAULT_PHOTO);
}
byte[] bytes = new byte[inputStream.available()];
inputStream.read(bytes, 0, inputStream.available());
response.setContentType(MediaType.IMAGE_JPEG_VALUE);
OutputStream out = response.getOutputStream();
out.write(bytes);
out.flush();
out.close();
} catch (Exception e) {
e.printStackTrace();
}
byte[] bytes = new byte[0];
try {
bytes = IOUtils.toByteArray(inputStream);
} catch (IOException e) {
throw new RuntimeException(e);
}
headers.setCacheControl(CacheControl.noCache().getHeaderValue());
ResponseEntity<byte[]> responseEntity = new ResponseEntity<>(bytes, headers, HttpStatus.OK);
return responseEntity;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,11 @@ public Result addAttachment(@CurrentSecurityContext SysUser requestor,
return Result.error(HttpStatus.BAD_REQUEST.value(), "Error fileType");
}
try {
String newFileName = FileUtil.getLegalFileName(attachment.getOriginalFilename());
String originalFilename = attachment.getOriginalFilename();
if (originalFilename.contains("..") || originalFilename.contains("/") || originalFilename.contains("\\")) {
throw new HydraLabRuntimeException("Invalid filename");
}
String newFileName = FileUtil.getLegalFileName(originalFilename);
String fileRelativeParent = FileUtil.getPathForToday();
String parentDir = CENTER_FILE_BASE_DIR + fileRelativeParent;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Path;
import java.nio.file.Paths;

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

Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath();
Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath();
if (!filePath.startsWith(publicFolder + File.separator)) {
throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path");
}

File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri);
if (!file.exists()) {
throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), String.format("File %s not exist!", fileUri));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ public Result getSmartTestGraphPhoto(@CurrentSecurityContext SysUser requestor,
}
try {
File graphZipFile = loadGraphFile(fileId);

if (node.contains("..") || node.contains("/") || node.contains("\\")) {
throw new HydraLabRuntimeException("Invalid node name");
}
File nodeFile = new File(graphZipFile.getParentFile().getAbsolutePath(), node + "/" + node + "-0.jpg");
if (!nodeFile.exists()) {
throw new HydraLabRuntimeException("Graph photo file not found");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -777,6 +779,12 @@ private JSONObject runT2CTest(TestTaskSpec testTaskSpec) {
Map<String, Integer> aggregateDeviceCountMap = new HashMap<>();
for (StorageFileInfo testJsonInfo : attachmentTestJsonList) {
Map<String, Integer> deviceCountMap = new HashMap<>();

Path publicFolder = Paths.get(CENTER_FILE_BASE_DIR).normalize().toAbsolutePath();
Path filePath = publicFolder.resolve(testJsonInfo.getBlobPath()).normalize().toAbsolutePath();
if (!filePath.startsWith(publicFolder + File.separator)) {
throw new HydraLabRuntimeException("Invalid blob path");
}
File testJsonFile = new File(CENTER_FILE_BASE_DIR, testJsonInfo.getBlobPath());
TestInfo testInfo;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.microsoft.hydralab.center.util;

import com.alibaba.fastjson.JSONObject;
import com.alibaba.fastjson.parser.ParserConfig;
import com.microsoft.hydralab.common.util.RestTemplateConfig;
import com.microsoft.hydralab.common.util.FileUtil;
import org.apache.commons.codec.binary.Base64;
Expand Down Expand Up @@ -95,6 +96,7 @@ public JSONObject decodeAccessToken(String accessToken) {
String[] pieces = accessToken.split("\\.");
String b64payload = pieces[1];
String jsonString = new String(Base64.decodeBase64(b64payload), FileUtil.UTF_8);
ParserConfig.getGlobalInstance().setSafeMode(true);
userInfo = JSONObject.parseObject(jsonString);
} catch (Exception e) {
e.printStackTrace();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Path;
import java.nio.file.Paths;

/**
* @author Li Shen
Expand All @@ -28,6 +30,11 @@ private LocalStorageIOUtil() {
}

public static void copyUploadedStreamToFile(InputStream inputStream, String fileUri) {
Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath();
Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath();
if (!filePath.startsWith(publicFolder + File.separator)) {
throw new HydraLabRuntimeException("Invalid file uri");
}
File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri);
File parentDirFile = new File(file.getParent());
if (!parentDirFile.exists() && !parentDirFile.mkdirs()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,11 @@ public File verifyAndSaveFile(@NotNull MultipartFile originFile, String parentDi
if (!parentDirFile.exists() && !parentDirFile.mkdirs()) {
throw new HydraLabRuntimeException(HttpStatus.INTERNAL_SERVER_ERROR.value(), "mkdirs failed!");
}
String filename = FileUtil.getLegalFileName(originFile.getOriginalFilename());
String originalFilename = originFile.getOriginalFilename();
if (originalFilename.contains("..") || originalFilename.contains("/") || originalFilename.contains("\\")) {
throw new HydraLabRuntimeException("Invalid filename");
}
String filename = FileUtil.getLegalFileName(originalFilename);
String fileSuffix = null;
boolean isMatch = false;
if (filename == null) {
Expand Down
2 changes: 1 addition & 1 deletion hercules/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def create_file(path, content):

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

Expand Down

0 comments on commit 916d2a6

Please sign in to comment.