1

So I have been given the task of fixing a path traversal problem in a basic Java web app, but I am quite stuck. We are meant to essentially make sure the code is secure, while maintaining functionality (which is the part i am struggling with)

So far I have looked online on how to fix the problems i am receiving, and i managed to fix them, but the bot that tests the code returns with a message saying the application no longer has functionality, but is secure.

The 2 errors I receive are the following:

1) PATH_TRAVERSAL_IN in FileDownload. java Source File FileDownload. java Class Name chatapp. FileDownload Method Name doGet Source Line 31

2) PT_RELATIVE_PATH_TRAVERSAL in FileDownload. java Source File FileDownload. java Class Name chatapp. FileDownload Method Name doGet Source Line 28

For reference this code is the original where it functions but it is not secure.

    private String DOWNLOAD_PATH = new File(".").getCanonicalPath() + 
     "/webapps/webapp/app/download";

    public FileDownload() throws IOException {
    }

    public void init() throws ServletException {
        //To Do
    }

    public void doGet(HttpServletRequest request,
                       HttpServletResponse response)
            throws ServletException, IOException
    {

     !!!String file = request.getParameter("file");
        String downloadPath = DOWNLOAD_PATH + "/" + file;
     !!!File downloadFile = new File(FilenameUtils.getName(downloadPath));

        if (downloadFile.exists()) {
            response.setContentType("application/octet-stream");
            response.setHeader("Content-disposition", "attachment; filename="+ downloadFile.getName());
            FileInputStream fis = new FileInputStream(downloadFile);
            byte[] data = new byte[(int) downloadFile.length()];
            fis.read(data);
            fis.close();

            OutputStream out = response.getOutputStream();
            out.write(data);
            out.flush();
        }
        else
            response.sendError(404);

    }

Does anyone have experience in fixing these sorts of problems? I am sort of confused

ismaeel ali
  • 43
  • 1
  • 7
  • Did you add `!!!` to indicate which lines are 29 and 31, or are those present in the original file? What is the error message you're getting? – Haem Nov 12 '19 at 07:54
  • @Haem just to show you guys what lines 29 and 31 are, they are not in the original file – ismaeel ali Nov 12 '19 at 09:08
  • @Haem do you know how to go about mending it? i've spent all my time since posting this trying to search a fix. I think i am dumb or something lol – ismaeel ali Nov 12 '19 at 13:33
  • You haven't posted the full error message so it's difficult to tell what's wrong. – Haem Nov 12 '19 at 14:06
  • @Haem all edited now – ismaeel ali Nov 12 '19 at 14:34
  • Oh, I misunderstood. I thought you were getting compilation/runtime errors, but those are warnings from some analysis tool. – Haem Nov 12 '19 at 14:40
  • Yeah, and I am not sure how to make the code secure, but also function the same. I can make it secure, but then the analysis tool says the code no longer does the same function as it used to when it was not secure – ismaeel ali Nov 12 '19 at 14:43
  • Does this answer your question? [Pass sonar's PT\_RELATIVE\_PATH\_TRAVERSAL in java](https://stackoverflow.com/questions/30909672/pass-sonars-pt-relative-path-traversal-in-java) https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN – Andreas Nov 13 '19 at 09:02

1 Answers1

0

Wikipedia's article on path traversal has a proposed method to prevent it:

  1. Process URI requests that do not result in a file request, e.g., executing a hook into user code, before continuing below.
  2. When a URI request for a file/directory is to be made, build a full path to the file/directory if it exists, and normalize all characters (e.g., %20 converted to spaces).
  3. It is assumed that a 'Document Root' fully qualified, normalized, path is known, and this string has a length ''N''. Assume that no files outside this directory can be served.
  4. Ensure that the first ''N'' characters of the fully qualified path to the requested file is exactly the same as the 'Document Root'.
  5. If so, allow the file to be returned.
  6. If not, return an error, since the request is clearly out of bounds from what the web-server should be allowed to serve.

So, you'd create a second file object from DOWNLOAD_PATH, then use getCanonicalPath to see that the path of the file to be downloaded starts with the path of the download directory.

With this done, you'd add a @SuppressWarnings annotation to the method to hide the warnings that are now properly handled.

Haem
  • 929
  • 6
  • 15
  • 31