As a security engineer, and like many people in security, I prefer bulletproof solutions to patches that fix only half of the problem.

But when you come across the same code that, despite being far from perfect, keeps preventing you from exploiting some code that should be exploitable, you have to realize: "That’s good enough."

Sometimes a simple solution that prevents 80% of vulnerabilities is better than no solution: "Perfect is the enemy of good."

You may wonder why I’m blogging about this. Once again, I came across the following pattern in Golang:

func InitializeBlog(router *httptreemux.TreeMux) {
  […]
  router.GET("/images/*filepath", imagesHandler)
  […]
}

func imagesHandler(w http.ResponseWriter, r *http.Request, 
                                          params map[string]string) {

  http.ServeFile(w, r, filepath.Join(filenames.ImagesFilepath, 
                                     params["filepath"]))
  return
}     

If you are not familiar with Golang, http.ServeFile() will serve the file based on the third argument.

Here, you may think that you have a clear directory traversal. Accessing /images/../../../../../../../etc/passwd will give you the precious content of /etc/passwd.

Unfortunately (for attackers), in 2016, Golang developers introduced a protection against directory traversal in http.ServeFile() with the commit 9b67a5de79af56541c48c95c6d7ddc8630e1d0dc.

They added a check to throw an error if they detect a ...

But what is interesting is that they don’t check the name string provided as part of the third argument to http.ServeFile(). This check is implemented on the URL path: r.URL.Path, with r *Request being the HTTP request.

Basically, the code we saw above is not exploitable not because the developers were careful or prevented directory traversal attacks. It is not exploitable because they put the path to the file in the URL:

  router.GET("/images/*filepath", imagesHandler)

And the interesting thing is that the more protection developers try to put in place, the more likely they are to enable the exploitation of this issue.

For example, developers may have decided to get rid of ; in the path provided:

func imagesHandler(w http.ResponseWriter, r *http.Request, 
                    params map[string]string) {

  path := filepath.Join(filenames.ImagesFilepath, params["filepath"])
  newPath := strings.Replace(path, ";", "", -1)

  http.ServeFile(w, r, newPath)
}

Or they may decide to normalize, encode or decode the path. Any modifications, really...

Those modifications can potentially allow attackers to bypass the protection provided by http.ServeFile() because they no longer need .. to exploit the issue. And since the protection applies to r.URL.Path and not to name, the protection does not prevent the exploitation.

But in most cases, developers do nothing, and this protection is just "Good enough."