From 72bc6932b017edd246c2e6eadd34758e7dee1d68 Mon Sep 17 00:00:00 2001
From: W-Mark Kubacki <wmark@hurrikane.de>
Date: Tue, 19 Apr 2016 00:54:25 +0200
Subject: [PATCH] browse: Jail the root directory using http.Dir()

---
 caddy/setup/browse.go            | 12 ++++++++++-
 middleware/browse/browse.go      | 34 ++++++++++++++++++--------------
 middleware/browse/browse_test.go |  8 ++++----
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/caddy/setup/browse.go b/caddy/setup/browse.go
index 663eae3e9..4d3eaf819 100644
--- a/caddy/setup/browse.go
+++ b/caddy/setup/browse.go
@@ -3,6 +3,7 @@ package setup
 import (
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"text/template"
 
 	"github.com/mholt/caddy/middleware"
@@ -17,7 +18,6 @@ func Browse(c *Controller) (middleware.Middleware, error) {
 	}
 
 	browse := browse.Browse{
-		Root:          c.Root,
 		Configs:       configs,
 		IgnoreIndexes: false,
 	}
@@ -50,6 +50,16 @@ func browseParse(c *Controller) ([]browse.Config, error) {
 		} else {
 			bc.PathScope = "/"
 		}
+		bc.Root = http.Dir(c.Root)
+		theRoot, err := bc.Root.Open("/") // catch a missing path early
+		if err != nil {
+			return configs, err
+		}
+		defer theRoot.Close()
+		_, err = theRoot.Readdir(-1)
+		if err != nil {
+			return configs, err
+		}
 
 		// Second argument would be the template file to use
 		var tplText string
diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go
index d75b2acad..62e4b1684 100644
--- a/middleware/browse/browse.go
+++ b/middleware/browse/browse.go
@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"os"
 	"path"
-	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -24,7 +23,6 @@ import (
 // directories in the given paths are specified.
 type Browse struct {
 	Next          middleware.Handler
-	Root          string
 	Configs       []Config
 	IgnoreIndexes bool
 }
@@ -32,6 +30,7 @@ type Browse struct {
 // Config is a configuration for browsing in a particular path.
 type Config struct {
 	PathScope string
+	Root      http.FileSystem
 	Variables interface{}
 	Template  *template.Template
 }
@@ -247,8 +246,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 inScope:
 
 	// Browse works on existing directories; delegate everything else
-	requestedFilepath := filepath.Join(b.Root, r.URL.Path)
-	info, err := os.Stat(requestedFilepath)
+	requestedFilepath, err := bc.Root.Open(r.URL.Path)
 	if err != nil {
 		switch {
 		case os.IsPermission(err):
@@ -259,6 +257,19 @@ inScope:
 			return b.Next.ServeHTTP(w, r)
 		}
 	}
+	defer requestedFilepath.Close()
+
+	info, err := requestedFilepath.Stat()
+	if err != nil {
+		switch {
+		case os.IsPermission(err):
+			return http.StatusForbidden, err
+		case os.IsExist(err):
+			return http.StatusGone, err
+		default:
+			return b.Next.ServeHTTP(w, r)
+		}
+	}
 	if !info.IsDir() {
 		return b.Next.ServeHTTP(w, r)
 	}
@@ -283,15 +294,8 @@ inScope:
 	return b.ServeListing(w, r, requestedFilepath, bc)
 }
 
-func (b Browse) loadDirectoryContents(requestedFilepath, urlPath string) (*Listing, bool, error) {
-	// Load directory contents
-	file, err := os.Open(requestedFilepath)
-	if err != nil {
-		return nil, false, err
-	}
-	defer file.Close()
-
-	files, err := file.Readdir(-1)
+func (b Browse) loadDirectoryContents(requestedFilepath http.File, urlPath string) (*Listing, bool, error) {
+	files, err := requestedFilepath.Readdir(-1)
 	if err != nil {
 		return nil, false, err
 	}
@@ -351,7 +355,7 @@ func (b Browse) handleSortOrder(w http.ResponseWriter, r *http.Request, scope st
 }
 
 // ServeListing returns a formatted view of 'requestedFilepath' contents'.
-func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFilepath string, bc *Config) (int, error) {
+func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFilepath http.File, bc *Config) (int, error) {
 	listing, containsIndex, err := b.loadDirectoryContents(requestedFilepath, r.URL.Path)
 	if err != nil {
 		switch {
@@ -367,7 +371,7 @@ func (b Browse) ServeListing(w http.ResponseWriter, r *http.Request, requestedFi
 		return b.Next.ServeHTTP(w, r)
 	}
 	listing.Context = middleware.Context{
-		Root: http.Dir(b.Root),
+		Root: bc.Root,
 		Req:  r,
 		URL:  r.URL,
 	}
diff --git a/middleware/browse/browse_test.go b/middleware/browse/browse_test.go
index 0363d2f41..161498f43 100644
--- a/middleware/browse/browse_test.go
+++ b/middleware/browse/browse_test.go
@@ -114,10 +114,10 @@ func TestBrowseHTTPMethods(t *testing.T) {
 		Next: middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 			return http.StatusTeapot, nil // not t.Fatalf, or we will not see what other methods yield
 		}),
-		Root: "./testdata",
 		Configs: []Config{
 			{
 				PathScope: "/photos",
+				Root:      http.Dir("./testdata"),
 				Template:  tmpl,
 			},
 		},
@@ -153,10 +153,10 @@ func TestBrowseTemplate(t *testing.T) {
 			t.Fatalf("Next shouldn't be called")
 			return 0, nil
 		}),
-		Root: "./testdata",
 		Configs: []Config{
 			{
 				PathScope: "/photos",
+				Root:      http.Dir("./testdata"),
 				Template:  tmpl,
 			},
 		},
@@ -208,16 +208,16 @@ func TestBrowseJson(t *testing.T) {
 			t.Fatalf("Next shouldn't be called")
 			return 0, nil
 		}),
-		Root: "./testdata",
 		Configs: []Config{
 			{
 				PathScope: "/photos/",
+				Root:      http.Dir("./testdata"),
 			},
 		},
 	}
 
 	//Getting the listing from the ./testdata/photos, the listing returned will be used to validate test results
-	testDataPath := b.Root + "/photos/"
+	testDataPath := filepath.Join("./testdata", "photos")
 	file, err := os.Open(testDataPath)
 	if err != nil {
 		if os.IsPermission(err) {