Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat (limited to 'workhorse/internal/staticpages')
-rw-r--r--workhorse/internal/staticpages/deploy_page_test.go4
-rw-r--r--workhorse/internal/staticpages/error_pages_test.go14
-rw-r--r--workhorse/internal/staticpages/servefile.go47
-rw-r--r--workhorse/internal/staticpages/servefile_test.go46
-rw-r--r--workhorse/internal/staticpages/static.go1
-rw-r--r--workhorse/internal/staticpages/testdata/file11
-rw-r--r--workhorse/internal/staticpages/testdata/uploads/file21
7 files changed, 85 insertions, 29 deletions
diff --git a/workhorse/internal/staticpages/deploy_page_test.go b/workhorse/internal/staticpages/deploy_page_test.go
index 4b081e73a97..9bc0a082144 100644
--- a/workhorse/internal/staticpages/deploy_page_test.go
+++ b/workhorse/internal/staticpages/deploy_page_test.go
@@ -23,7 +23,7 @@ func TestIfNoDeployPageExist(t *testing.T) {
w := httptest.NewRecorder()
executed := false
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.DeployPage(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
executed = true
})).ServeHTTP(w, nil)
@@ -45,7 +45,7 @@ func TestIfDeployPageExist(t *testing.T) {
w := httptest.NewRecorder()
executed := false
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.DeployPage(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
executed = true
})).ServeHTTP(w, nil)
diff --git a/workhorse/internal/staticpages/error_pages_test.go b/workhorse/internal/staticpages/error_pages_test.go
index 05ec06cd429..ab29e187c8e 100644
--- a/workhorse/internal/staticpages/error_pages_test.go
+++ b/workhorse/internal/staticpages/error_pages_test.go
@@ -32,7 +32,7 @@ func TestIfErrorPageIsPresented(t *testing.T) {
require.NoError(t, err)
require.Equal(t, len(upstreamBody), n, "bytes written")
})
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ErrorPagesUnless(false, ErrorFormatHTML, h).ServeHTTP(w, nil)
w.Flush()
@@ -54,7 +54,7 @@ func TestIfErrorPassedIfNoErrorPageIsFound(t *testing.T) {
w.WriteHeader(404)
fmt.Fprint(w, errorResponse)
})
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ErrorPagesUnless(false, ErrorFormatHTML, h).ServeHTTP(w, nil)
w.Flush()
@@ -78,7 +78,7 @@ func TestIfErrorPageIsIgnoredInDevelopment(t *testing.T) {
w.WriteHeader(500)
fmt.Fprint(w, serverError)
})
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ErrorPagesUnless(true, ErrorFormatHTML, h).ServeHTTP(w, nil)
w.Flush()
require.Equal(t, 500, w.Code)
@@ -102,7 +102,7 @@ func TestIfErrorPageIsIgnoredIfCustomError(t *testing.T) {
w.WriteHeader(500)
fmt.Fprint(w, serverError)
})
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ErrorPagesUnless(false, ErrorFormatHTML, h).ServeHTTP(w, nil)
w.Flush()
require.Equal(t, 500, w.Code)
@@ -137,7 +137,7 @@ func TestErrorPageInterceptedByContentType(t *testing.T) {
w.WriteHeader(500)
fmt.Fprint(w, serverError)
})
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ErrorPagesUnless(false, ErrorFormatHTML, h).ServeHTTP(w, nil)
w.Flush()
require.Equal(t, 500, w.Code)
@@ -161,7 +161,7 @@ func TestIfErrorPageIsPresentedJSON(t *testing.T) {
require.NoError(t, err)
require.Equal(t, len(upstreamBody), n, "bytes written")
})
- st := &Static{""}
+ st := &Static{}
st.ErrorPagesUnless(false, ErrorFormatJSON, h).ServeHTTP(w, nil)
w.Flush()
@@ -181,7 +181,7 @@ func TestIfErrorPageIsPresentedText(t *testing.T) {
require.NoError(t, err)
require.Equal(t, len(upstreamBody), n, "bytes written")
})
- st := &Static{""}
+ st := &Static{}
st.ErrorPagesUnless(false, ErrorFormatText, h).ServeHTTP(w, nil)
w.Flush()
diff --git a/workhorse/internal/staticpages/servefile.go b/workhorse/internal/staticpages/servefile.go
index c98bc030bc2..7e39f919fa6 100644
--- a/workhorse/internal/staticpages/servefile.go
+++ b/workhorse/internal/staticpages/servefile.go
@@ -1,16 +1,18 @@
package staticpages
import (
+ "errors"
+ "fmt"
"net/http"
"os"
"path/filepath"
"strings"
"time"
- "gitlab.com/gitlab-org/labkit/log"
"gitlab.com/gitlab-org/labkit/mask"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+ "gitlab.com/gitlab-org/gitlab-workhorse/internal/log"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/urlprefix"
)
@@ -26,21 +28,28 @@ const (
// upstream.
func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoundHandler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- file := filepath.Join(s.DocumentRoot, prefix.Strip(r.URL.Path))
+ if notFoundHandler == nil {
+ notFoundHandler = http.HandlerFunc(http.NotFound)
+ }
+
+ // We intentionally use r.URL.Path instead of r.URL.EscaptedPath() below.
+ // This is to make it possible to serve static files with e.g. a space
+ // %20 in their name.
+ relativePath, err := s.validatePath(prefix.Strip(r.URL.Path))
+ if err != nil {
+ notFoundHandler.ServeHTTP(w, r)
+ return
+ }
- // The filepath.Join does Clean traversing directories up
+ file := filepath.Join(s.DocumentRoot, relativePath)
if !strings.HasPrefix(file, s.DocumentRoot) {
- helper.Fail500(w, r, &os.PathError{
- Op: "open",
- Path: file,
- Err: os.ErrInvalid,
- })
+ log.WithRequest(r).WithError(errPathTraversal).Error()
+ notFoundHandler.ServeHTTP(w, r)
return
}
var content *os.File
var fi os.FileInfo
- var err error
// Serve pre-gzipped assets
if acceptEncoding := r.Header.Get("Accept-Encoding"); strings.Contains(acceptEncoding, "gzip") {
@@ -55,11 +64,7 @@ func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoun
content, fi, err = helper.OpenFile(file)
}
if err != nil {
- if notFoundHandler != nil {
- notFoundHandler.ServeHTTP(w, r)
- } else {
- http.NotFound(w, r)
- }
+ notFoundHandler.ServeHTTP(w, r)
return
}
defer content.Close()
@@ -82,3 +87,17 @@ func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoun
http.ServeContent(w, r, filepath.Base(file), fi.ModTime(), content)
})
}
+
+var errPathTraversal = errors.New("path traversal")
+
+func (s *Static) validatePath(filename string) (string, error) {
+ filename = filepath.Clean(filename)
+
+ for _, exc := range s.Exclude {
+ if strings.HasPrefix(filename, exc) {
+ return "", fmt.Errorf("file is excluded: %s", exc)
+ }
+ }
+
+ return filename, nil
+}
diff --git a/workhorse/internal/staticpages/servefile_test.go b/workhorse/internal/staticpages/servefile_test.go
index e136b876298..314547b8a57 100644
--- a/workhorse/internal/staticpages/servefile_test.go
+++ b/workhorse/internal/staticpages/servefile_test.go
@@ -20,7 +20,7 @@ func TestServingNonExistingFile(t *testing.T) {
httpRequest, _ := http.NewRequest("GET", "/file", nil)
w := httptest.NewRecorder()
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
require.Equal(t, 404, w.Code)
}
@@ -34,7 +34,7 @@ func TestServingDirectory(t *testing.T) {
httpRequest, _ := http.NewRequest("GET", "/file", nil)
w := httptest.NewRecorder()
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
require.Equal(t, 404, w.Code)
}
@@ -44,7 +44,7 @@ func TestServingMalformedUri(t *testing.T) {
httpRequest, _ := http.NewRequest("GET", "/../../../static/file", nil)
w := httptest.NewRecorder()
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
require.Equal(t, 404, w.Code)
}
@@ -54,7 +54,7 @@ func TestExecutingHandlerWhenNoFileFound(t *testing.T) {
httpRequest, _ := http.NewRequest("GET", "/file", nil)
executed := false
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
executed = (r == httpRequest)
})).ServeHTTP(nil, httpRequest)
@@ -76,7 +76,7 @@ func TestServingTheActualFile(t *testing.T) {
ioutil.WriteFile(filepath.Join(dir, "file"), []byte(fileContent), 0600)
w := httptest.NewRecorder()
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
require.Equal(t, 200, w.Code)
if w.Body.String() != fileContent {
@@ -84,6 +84,40 @@ func TestServingTheActualFile(t *testing.T) {
}
}
+func TestExcludedPaths(t *testing.T) {
+ testCases := []struct {
+ desc string
+ path string
+ found bool
+ contents string
+ }{
+ {"allowed file", "/file1", true, "contents1"},
+ {"path traversal is allowed", "/uploads/../file1", true, "contents1"},
+ {"files in /uploads/ are invisible", "/uploads/file2", false, ""},
+ {"cannot use path traversal to get to /uploads/", "/foobar/../uploads/file2", false, ""},
+ {"cannot use escaped path traversal to get to /uploads/", "/foobar%2f%2e%2e%2fuploads/file2", false, ""},
+ {"cannot use double escaped path traversal to get to /uploads/", "/foobar%252f%252e%252e%252fuploads/file2", false, ""},
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.desc, func(t *testing.T) {
+ httpRequest, err := http.NewRequest("GET", tc.path, nil)
+ require.NoError(t, err)
+
+ w := httptest.NewRecorder()
+ st := &Static{DocumentRoot: "testdata", Exclude: []string{"/uploads/"}}
+ st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
+
+ if tc.found {
+ require.Equal(t, 200, w.Code)
+ require.Equal(t, tc.contents, w.Body.String())
+ } else {
+ require.Equal(t, 404, w.Code)
+ }
+ })
+ }
+}
+
func testServingThePregzippedFile(t *testing.T, enableGzip bool) {
dir, err := ioutil.TempDir("", "deploy")
if err != nil {
@@ -108,7 +142,7 @@ func testServingThePregzippedFile(t *testing.T, enableGzip bool) {
ioutil.WriteFile(filepath.Join(dir, "file"), []byte(fileContent), 0600)
w := httptest.NewRecorder()
- st := &Static{dir}
+ st := &Static{DocumentRoot: dir}
st.ServeExisting("/", CacheDisabled, nil).ServeHTTP(w, httpRequest)
require.Equal(t, 200, w.Code)
if enableGzip {
diff --git a/workhorse/internal/staticpages/static.go b/workhorse/internal/staticpages/static.go
index b42351f15f5..5b804e4d644 100644
--- a/workhorse/internal/staticpages/static.go
+++ b/workhorse/internal/staticpages/static.go
@@ -2,4 +2,5 @@ package staticpages
type Static struct {
DocumentRoot string
+ Exclude []string
}
diff --git a/workhorse/internal/staticpages/testdata/file1 b/workhorse/internal/staticpages/testdata/file1
new file mode 100644
index 00000000000..146edcbe0a3
--- /dev/null
+++ b/workhorse/internal/staticpages/testdata/file1
@@ -0,0 +1 @@
+contents1 \ No newline at end of file
diff --git a/workhorse/internal/staticpages/testdata/uploads/file2 b/workhorse/internal/staticpages/testdata/uploads/file2
new file mode 100644
index 00000000000..c061beb8592
--- /dev/null
+++ b/workhorse/internal/staticpages/testdata/uploads/file2
@@ -0,0 +1 @@
+contents2 \ No newline at end of file