Skip to content
Permalink
Browse files Browse the repository at this point in the history
OC-17139: code changes after code review
  • Loading branch information
Tao Li committed Feb 3, 2022
1 parent bd363d4 commit 6f864e8
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 30 deletions.
Expand Up @@ -95,14 +95,22 @@ public void processRequest() throws Exception {
excelFile = new File(dir,excelFileName);
// backwards compat
File oldExcelFile = new File(dir, oldExcelFileName);

if (oldExcelFile.exists() && oldExcelFile.length() > 0) {
if (!excelFile.exists() || excelFile.length() <= 0) {
// if the old name exists and the new name does not...
excelFile = oldExcelFile;
excelFileName = oldExcelFileName;
}
}

String canonicalPath1= excelFile.getCanonicalPath();
String canonicalPath2= oldExcelFile.getCanonicalPath();
if (canonicalPath1.startsWith(dir) && canonicalPath2.startsWith(dir)) {
if (oldExcelFile.exists() && oldExcelFile.length() > 0) {
if (!excelFile.exists() || excelFile.length() <= 0) {
// if the old name exists and the new name does not...
excelFile = oldExcelFile;
excelFileName = oldExcelFileName;
}
}
}else {
addPageMessage(respage.getString("the_excel_is_not_available_on_server_contact"));
forwardPage(Page.CRF_LIST_SERVLET);
}


}
logger.info("looking for : " + excelFile.getName());
Expand Down
Expand Up @@ -70,17 +70,23 @@ public void processRequest() throws Exception {
String filePathName = "";
String fileName = fp.getString("fileName");
File f = new File(fileName);

if(fileName != null && fileName.indexOf("..") > -1) {
throw new RuntimeException("Traversal attempt - absolute path not allowed " + fileName);
}


if (fileName != null && fileName.length() > 0) {
int parentStudyId = currentStudy.getParentStudyId();
int parentStudyId = currentStudy.getParentStudyId();
String testPath = Utils.getAttachedFileRootPath();
String tail = File.separator + f.getName();
String testName = testPath + currentStudy.getOid() + tail;
File temp = new File(testName);

String filePath = testPath + currentStudy.getOid() +File.separator;
File temp = new File(filePath,f.getName());
String canonicalPath= temp.getCanonicalPath();

if (canonicalPath.startsWith(filePath)) {
;
}else {
throw new RuntimeException("Traversal attempt - file path not allowed " + fileName);
}

if (temp.exists()) {
filePathName = testName;
logger.info(currentStudy.getName() + " existing filePathName=" + filePathName);
Expand Down
Expand Up @@ -127,13 +127,20 @@ public void getLogFile(@PathVariable("filename") String fileName, HttpServletRes
InputStream inputStream = null;
try {
//Validate/Sanitize user input filename using a standard library, prevent from path traversal
String logFileName = getFilePath() + File.separator + FilenameUtils.getName(fileName);
File fileToDownload = new File(logFileName);
inputStream = new FileInputStream(fileToDownload);
response.setContentType("application/force-download");
response.setHeader("Content-Disposition", "attachment; filename=" + fileName);
IOUtils.copy(inputStream, response.getOutputStream());
response.flushBuffer();
String logFilePath = getFilePath() + File.separator;
File fileToDownload = new File(logFilePath, fileName);
String canonicalPath= fileToDownload.getCanonicalPath();

if (canonicalPath.startsWith(logFilePath)) {
inputStream = new FileInputStream(fileToDownload);
response.setContentType("application/force-download");
response.setHeader("Content-Disposition", "attachment; filename=" + fileName);
IOUtils.copy(inputStream, response.getOutputStream());
response.flushBuffer();
}else {
throw new RuntimeException("Traversal attempt - file path not allowed " + fileName);
}

} catch (Exception e) {
logger.debug("Request could not be completed at this moment. Please try again.");
logger.debug(e.getStackTrace().toString());
Expand Down
Expand Up @@ -27,7 +27,6 @@
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -194,16 +193,25 @@ private boolean mayProceed(String studyOid, StudySubjectBean ssBean) throws Exce
return accessPermission;
}

public static String getAttachedFilePath(String inputStudyOid) {
// Using a standard library to validate/Sanitize user inputs which will be used in path expression to prevent from path traversal
String studyOid = FilenameUtils.getName(inputStudyOid);
public static String getAttachedFilePath(String studyOid) throws Exception {
String attachedFilePath = CoreResources.getField("attached_file_location");
if (attachedFilePath == null || attachedFilePath.length() <= 0) {
attachedFilePath = CoreResources.getField("filePath") + "attached_files" + File.separator + studyOid + File.separator;
} else {
attachedFilePath += studyOid + File.separator;
attachedFilePath = CoreResources.getField("filePath") + "attached_files" + File.separator;
}
File tempFile = new File(attachedFilePath,studyOid);
String canonicalPath= tempFile.getCanonicalPath();

if (canonicalPath.startsWith(attachedFilePath)) {
if (attachedFilePath == null || attachedFilePath.length() <= 0) {
attachedFilePath = CoreResources.getField("filePath") + "attached_files" + File.separator + studyOid + File.separator;
} else {
attachedFilePath += studyOid + File.separator;
}
return attachedFilePath;
}else {
throw new RuntimeException("Traversal attempt - file path not allowed " + studyOid);
}
return attachedFilePath;

}

private File processUploadedFile(FileItem item, String dirToSaveUploadedFileIn) {
Expand Down

0 comments on commit 6f864e8

Please sign in to comment.