address potential path traversal raised by sync

ncats · Jun 13, 2024 · 8c9486d · 8c9486d
1 parent 9f8261b
commit 8c9486d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 3 deletions.
diff --git a/...tances-core/src/main/java/gsrs/module/substance/controllers/LegacyGinasAppController.java b/...tances-core/src/main/java/gsrs/module/substance/controllers/LegacyGinasAppController.java
@@ -34,6 +34,8 @@
 import javax.servlet.http.HttpServletRequest;
 import java.io.IOException;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
@@ -96,6 +98,16 @@ public ResponseEntity<Object> uploadPayload(
         return payloadController.handleFileUpload(file, queryParameters);
 
     }
+
+    // are the slashes right, needed?
+    public final Pattern ID_PATTERN = Pattern.compile("[a-f0-9\\-]+");
+
+    private boolean checkId(String id)  {
+        if (id == null) return false;
+        Matcher matcher = ID_PATTERN.matcher(id);
+        return matcher.find();
+    }
+
     //GET         /export/$id<[a-f0-9\-]+>.$format<(mol|sdf|smi|smiles|fas)>
     // ix.ginas.controllers.GinasApp.structureExport(id: String, format: String, context: String ?= null)
     @GetMapping({"export/{id:[a-f0-9\\-]+}.{format}","/ginas/app/export/{id:[a-f0-9\\-]+}.{format}"})
@@ -106,6 +118,10 @@ public Object exportStructure(@PathVariable String id, @PathVariable String form
                               @RequestParam(value = "stereo", required = false, defaultValue = "") Boolean stereo,
                               HttpServletRequest httpRequest, RedirectAttributes attributes,
                                   @RequestParam Map<String, String> queryParameters){
+        if (!checkId(id)) {
+            // This is to satisfy Snyk security analysis, probably never gets here if annotation works.
+            return gsrsControllerConfiguration.handleBadRequest(400, "Badly formatted id in url placeholder", null);
+        }
         if("mol".equalsIgnoreCase(format) || "sdf".equalsIgnoreCase(format) ||
                 "smi".equalsIgnoreCase(format) ||  "smiles".equalsIgnoreCase(format) ) {
             //TODO: use cache where possible here

diff --git a/...substance-extension/src/main/java/example/ncats/controllers/NCATSSubstanceController.java b/...substance-extension/src/main/java/example/ncats/controllers/NCATSSubstanceController.java
@@ -1,6 +1,7 @@
 package example.ncats.controllers;
 
 import gsrs.*;
+import gsrs.config.FilePathParserUtils;
 import gsrs.controller.GetGsrsRestApiMapping;
 import gsrs.controller.GsrsRestApiController;
 import gsrs.controller.PostGsrsRestApiMapping;
@@ -86,14 +87,17 @@ public String getFieldInfo() {
     }
 
     public static File multipartToFile(MultipartFile multipart, String fileName) throws IllegalStateException, IOException {
-        File convFile = new File(System.getProperty("java.io.tmpdir")+"/"+fileName);
+        String basePath =System.getProperty("java.io.tmpdir");
+        FilePathParserUtils.failOnBadPathResolution(basePath, fileName);
+        File convFile = new File(basePath +"/"+fileName);
         multipart.transferTo(convFile);
         return convFile;
     }
 
     public static File stringToFile(String data, String fileName) throws IllegalStateException, IOException {
-
-        File convFile = new File(System.getProperty("java.io.tmpdir")+"/"+fileName);
+        String basePath =System.getProperty("java.io.tmpdir");
+        FilePathParserUtils.failOnBadPathResolution(basePath, fileName);
+        File convFile = new File(basePath+"/"+fileName);
         try(FileWriter writer = new FileWriter(convFile.getAbsoluteFile())) {
             writer.write(data);
         }