3

I am working with some coverity issues in my source code . Here i am using the code like ,

    filePath = properties.getProperty("DO.LIB.LOC");
    String fileName = (String) request.getParameter("read");
    filePath += "/" + fileName;
    downloadResultSet.flushFile(filePath, response, 
    fileName.substring(fileName.lastIndexOf(".") + 1));

In my coveiry scanned tool found an error like "CID 38762 (#1 of 2): Filesystem path, filename, or URI manipulation (PATH_MANIPULATION) 2. sink: Constructing a path or URI using the tainted value filePath. This may allow an attacker to access, modify, or test the existence of critical or sensitive files. The value is used unsafely in bytecode, which cannot be displayed."

File Path defined in an external property file and file name takes from request .

I am using the same code in different java file for file upload ,delete ,download functionalities . How can i avoid these kind of vulnerabilities from my code . Can any one help me on this ?

user3518223
  • 155
  • 1
  • 5
  • 13

2 Answers2

0

You should use the Path type instead of String (https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/Path.html)

it would look like something like this:

 Path path = Paths.get(properties.getProperty("DO.LIB.LOC"));
 path = path.resolve(Paths.get(request.getParameter("read")));
fan
  • 2,234
  • 1
  • 23
  • 24
0

Something like this will work:

    List<String> INVALID_FILE_NAME_CHARACTERS = Arrays.asList("..", ":", "/", "\\");
    String filePath = properties.getProperty("DO.LIB.LOC");
    String fileName = (String) request.getParameter("read");

    // Validate inputs
    if(filePath==null || fileName==null) return;
    for(String character : B2BProfileConstant.INVALID_FILE_NAME_CHARACTERS) {
        if(fileName.contains(character)) return;
    }

    Path path = Paths.get(filePath).resolve(fileName).normalize();
    String finalPath = path.toString();
V.Aggarwal
  • 557
  • 4
  • 12
  • I doubt it. You add a null check, and then return `null` when we don't know if the caller can handle that. But the real problem the linter is complaining about is that the code uses unsanitized, tainted user input. If someone enters `../../../../../etc/passwd` (with enough `../`s) they can eventually download the system's passwords file. – Robert Apr 18 '23 at 15:31
  • Thanks for pointing this out. Updated the code a bit. – V.Aggarwal Apr 21 '23 at 09:26