Commit aedbb005 authored by Steve Azzopardi's avatar Steve Azzopardi

Merge branch 'security-1-1-fix-toctou-race' into '1-1-stable'

[1.1] Fix TOCTOU race condition when serving files

See merge request gitlab/gitlab-pages!5
parents 5cffa835 d4586dad
Pipeline #38322158 passed with stage
in 1 minute and 43 seconds
image: golang:1.8
.test: &test
script:
- make setup
- make verify
- make cover
- echo "Running all tests without daemonizing..."
- make test
- echo "Running just the acceptance tests daemonized (tmpdir)...."
......@@ -14,9 +9,19 @@ image: golang:1.8
artifacts:
paths:
- bin/gitlab-pages
verify:
image: golang:1.11
script:
- make setup
- make verify
- make cover
artifacts:
paths:
- coverage.html
test:1.8:
image: golang:1.8
<<: *test
test:1.9:
......@@ -26,3 +31,7 @@ test:1.9:
test:1.10:
image: golang:1.10
<<: *test
test:1.11:
image: golang:1.11
<<: *test
v 1.1.1
- Fix TOCTOU race condition when serving files
v 1.1.0
- Fix HTTP to HTTPS redirection not working for default domains !106
- Log duplicate domain names !107
......
......@@ -14,6 +14,8 @@ import (
"sync"
"time"
"golang.org/x/sys/unix"
"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
"gitlab.com/gitlab-org/gitlab-pages/internal/httputil"
)
......@@ -131,7 +133,7 @@ func (d *D) IsHTTPSOnly(r *http.Request) bool {
func (d *D) serveFile(w http.ResponseWriter, r *http.Request, origPath string) error {
fullPath := handleGZip(w, r, origPath)
file, err := os.Open(fullPath)
file, err := openNoFollow(fullPath)
if err != nil {
return err
}
......@@ -155,7 +157,7 @@ func (d *D) serveCustomFile(w http.ResponseWriter, r *http.Request, code int, or
fullPath := handleGZip(w, r, origPath)
// Open and serve content of file
file, err := os.Open(fullPath)
file, err := openNoFollow(fullPath)
if err != nil {
return err
}
......@@ -330,3 +332,7 @@ func (d *D) ServeHTTP(w http.ResponseWriter, r *http.Request) {
func endsWithSlash(path string) bool {
return strings.HasSuffix(path, "/")
}
func openNoFollow(path string) (*os.File, error) {
return os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW, 0)
}
......@@ -361,6 +361,29 @@ func TestCacheControlHeaders(t *testing.T) {
assert.WithinDuration(t, now.UTC().Add(10*time.Minute), expiresTime.UTC(), time.Minute)
}
func TestOpenNoFollow(t *testing.T) {
tmpfile, err := ioutil.TempFile("", "link-test")
require.NoError(t, err)
defer tmpfile.Close()
orig := tmpfile.Name()
softLink := orig + ".link"
defer os.Remove(orig)
source, err := openNoFollow(orig)
require.NoError(t, err)
require.NotNil(t, source)
defer source.Close()
err = os.Symlink(orig, softLink)
require.NoError(t, err)
defer os.Remove(softLink)
link, err := openNoFollow(softLink)
require.Error(t, err)
require.Nil(t, link)
}
var chdirSet = false
func setUpTests() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment