Merge branch 'fix-toctou-1-3' into '1-3-stable'v1.3.11-3-stable

[1.3] Fix TOCTOU race condition when serving files

See merge request gitlab/gitlab-pages!7

4 files changed, 35 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cb69cb8f..80fc771d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+v 1.3.1
+- Fix TOCTOU race condition when serving files
+
 v 1.3.0
 - Allow the maximum connection concurrency to be set !117
 - Update Prometheus vendoring to v0.9 !116
diff --git a/VERSION b/VERSION
index f0bb29e7..3a3cd8cc 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.3.0
+1.3.1
diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 261707cf..2c4f4e29 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -15,6 +15,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"
 )
@@ -231,7 +233,7 @@ func (d *D) HasProject(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
 	}
@@ -257,7 +259,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
 	}
@@ -455,3 +457,7 @@ func (d *D) ServeNotFoundHTTP(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)
+}
diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 890bc6af..7544f501 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -397,6 +397,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() {