diff options
author | Nick Thomas <nick@gitlab.com> | 2018-01-31 22:42:08 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-02-19 17:29:12 +0300 |
commit | fbf87a29cf31ade0244f8d98729dda89c29a464c (patch) | |
tree | 2de796422f73ae2e3cee387017b014cd045575fc /domain.go | |
parent | a57de7adc1288ceafb3e6dcd50a3f0be1cec0028 (diff) |
Serve a secure redirect in case of accessing /foo
When a request's path resolved to a directory on disk and lacked a trailing
slash character, we issue a 302 Found redirect to the request's path, plus the
missing trailing slash. However, some request paths are valid absolute URIs
(particularly protocol-neutral //example.com URIs), so this was an open redirect
vulnerability.
This problem is avoided by generating a URI from the actual location of a file
that we want to present.
There were also numerous potential bypasses of other security checks for
inferred index.html files and custom error pages; this commit closes these
holes at the same time by recursively running the checks if necessary.
Diffstat (limited to 'domain.go')
-rw-r--r-- | domain.go | 111 |
1 files changed, 62 insertions, 49 deletions
@@ -17,6 +17,15 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/httputil" ) +type locationDirectoryError struct { + FullPath string + RelativePath string +} + +func (l *locationDirectoryError) Error() string { + return "location error accessing directory where file expected" +} + type domain struct { Group string Project string @@ -115,20 +124,45 @@ func (d *domain) serveCustomFile(w http.ResponseWriter, r *http.Request, code in return nil } -func (d *domain) resolvePath(projectName string, subPath ...string) (fullPath string, err error) { +// Resolve the HTTP request to a path on disk, converting requests for +// directories to requests for index.html inside the directory if appropriate. +func (d *domain) resolvePath(projectName string, subPath ...string) (string, error) { publicPath := filepath.Join(d.Group, projectName, "public") - fullPath = filepath.Join(publicPath, filepath.Join(subPath...)) - fullPath, err = filepath.EvalSymlinks(fullPath) + // Don't use filepath.Join as cleans the path, + // where we want to traverse full path as supplied by user + // (including ..) + testPath := publicPath + "/" + strings.Join(subPath, "/") + fullPath, err := filepath.EvalSymlinks(testPath) if err != nil { - return + return "", err } + // The requested path resolved to somewhere outside of the public/ directory if !strings.HasPrefix(fullPath, publicPath+"/") && fullPath != publicPath { - err = fmt.Errorf("%q should be in %q", fullPath, publicPath) - return + return "", fmt.Errorf("%q should be in %q", fullPath, publicPath) + } + + fi, err := os.Lstat(fullPath) + if err != nil { + return "", err + } + + // The requested path is a directory, so try index.html via recursion + if fi.IsDir() { + return "", &locationDirectoryError{ + FullPath: fullPath, + RelativePath: strings.TrimPrefix(fullPath, publicPath), + } + } + + // The file exists, but is not a supported type to serve. Perhaps a block + // special device or something else that may be a security risk. + if !fi.Mode().IsRegular() { + return "", fmt.Errorf("%s: is not a regular file", fullPath) } - return + + return fullPath, nil } func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName string) error { @@ -137,12 +171,6 @@ func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName return err } - // Make sure that file is not symlink - fi, err := os.Lstat(page404) - if err != nil && !fi.Mode().IsRegular() { - return err - } - err = d.serveCustomFile(w, r, http.StatusNotFound, page404) if err != nil { return err @@ -150,45 +178,30 @@ func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName return nil } -func (d *domain) checkPath(w http.ResponseWriter, r *http.Request, path string) (fullPath string, err error) { - fullPath = path - fi, err := os.Lstat(fullPath) - if err != nil { - return - } - - switch { - // If the URL doesn't end with /, send location to client - case fi.IsDir() && !endsWithSlash(r.URL.Path): - newURL := *r.URL - newURL.Path += "/" - http.Redirect(w, r, newURL.String(), 302) - - // If this is directory, we try the index.html - case fi.IsDir(): - fullPath = filepath.Join(fullPath, "index.html") - fi, err = os.Lstat(fullPath) - if err != nil { - return +func (d *domain) tryFile(w http.ResponseWriter, r *http.Request, projectName, pathSuffix string, subPath ...string) error { + fullPath, err := d.resolvePath(projectName, subPath...) + + if locationError, _ := err.(*locationDirectoryError); locationError != nil { + if endsWithSlash(r.URL.Path) { + fullPath, err = d.resolvePath(projectName, filepath.Join(subPath...), "index.html") + } else { + redirectPath := "//" + r.Host + "/" + if pathSuffix != "" { + redirectPath += pathSuffix + "/" + } + if locationError.RelativePath != "" { + redirectPath += strings.TrimPrefix(locationError.RelativePath, "/") + "/" + } + http.Redirect(w, r, redirectPath, 302) + return nil } - - // We don't allow to open the regular file - case !fi.Mode().IsRegular(): - err = fmt.Errorf("%s: is not a regular file", fullPath) } - return -} -func (d *domain) tryFile(w http.ResponseWriter, r *http.Request, projectName string, subPath ...string) error { - path, err := d.resolvePath(projectName, subPath...) if err != nil { return err } - path, err = d.checkPath(w, r, path) - if err != nil { - return err - } - return d.serveFile(w, r, path) + + return d.serveFile(w, r, fullPath) } func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { @@ -196,12 +209,12 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { split := strings.SplitN(r.URL.Path, "/", 3) // Try to serve file for http://group.example.com/subpath/... => /group/subpath/... - if len(split) >= 2 && d.tryFile(w, r, split[1], split[2:]...) == nil { + if len(split) >= 2 && d.tryFile(w, r, split[1], split[1], split[2:]...) == nil { return } // Try to serve file for http://group.example.com/... => /group/group.example.com/... - if r.Host != "" && d.tryFile(w, r, strings.ToLower(r.Host), r.URL.Path) == nil { + if r.Host != "" && d.tryFile(w, r, strings.ToLower(r.Host), "", r.URL.Path) == nil { return } @@ -221,7 +234,7 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) { // Try to serve file for http://host/... => /group/project/... - if d.tryFile(w, r, d.Project, r.URL.Path) == nil { + if d.tryFile(w, r, d.Project, "", r.URL.Path) == nil { return } |