Add latest changes from gitlab-org/security/gitlab@14-2-stable-ee

4 files changed, 45 insertions, 12 deletions

diff --git a/workhorse/internal/artifacts/artifacts_upload_test.go b/workhorse/internal/artifacts/artifacts_upload_test.go
index ce078c78559..488636043bc 100644
--- a/workhorse/internal/artifacts/artifacts_upload_test.go
+++ b/workhorse/internal/artifacts/artifacts_upload_test.go
@@ -270,7 +270,7 @@ func TestUploadHandlerForMultipleFiles(t *testing.T) {
 	require.NoError(t, s.writer.Close())
 
 	response := testUploadArtifacts(t, s.writer.FormDataContentType(), s.url, s.buffer)
-	require.Equal(t, http.StatusInternalServerError, response.Code)
+	require.Equal(t, http.StatusBadRequest, response.Code)
 }
 
 func TestUploadFormProcessing(t *testing.T) {
diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go
index 7945fa1f34d..79ebfe950c5 100644
--- a/workhorse/internal/upload/rewrite.go
+++ b/workhorse/internal/upload/rewrite.go
@@ -25,7 +25,10 @@ import (
 )
 
 // ErrInjectedClientParam means that the client sent a parameter that overrides one of our own fields
-var ErrInjectedClientParam = errors.New("injected client parameter")
+var (
+	ErrInjectedClientParam   = errors.New("injected client parameter")
+	ErrMultipleFilesUploaded = errors.New("upload request contains more than one file")
+)
 
 var (
 	multipartUploadRequests = promauto.NewCounterVec(
@@ -114,6 +117,10 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr
 }
 
 func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error {
+	if rew.filter.Count() > 0 {
+		return ErrMultipleFilesUploaded
+	}
+
 	multipartFiles.WithLabelValues(rew.filter.Name()).Inc()
 
 	filename := filepath.Base(p.FileName())
diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go
index 7a93fa4a9d8..8f4c8bb95f0 100644
--- a/workhorse/internal/upload/uploads.go
+++ b/workhorse/internal/upload/uploads.go
@@ -21,6 +21,7 @@ type MultipartFormProcessor interface {
 	ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error
 	Finalize(ctx context.Context) error
 	Name() string
+	Count() int
 }
 
 func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) {
@@ -34,6 +35,8 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, p
 		switch err {
 		case ErrInjectedClientParam:
 			helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest)
+		case ErrMultipleFilesUploaded:
+			helper.CaptureAndFail(w, r, err, "Uploading multiple files is not allowed", http.StatusBadRequest)
 		case http.ErrNotMultipart:
 			h.ServeHTTP(w, r)
 		case filestore.ErrEntityTooLarge:
diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go
index 5fae5cf7bcb..a655cabd2e7 100644
--- a/workhorse/internal/upload/uploads_test.go
+++ b/workhorse/internal/upload/uploads_test.go
@@ -18,7 +18,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/filestore"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/objectstore/test"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy"
@@ -28,11 +27,7 @@ import (
 
 var nilHandler = http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})
 
-type testFormProcessor struct{}
-
-func (a *testFormProcessor) ProcessFile(ctx context.Context, formName string, file *filestore.FileHandler, writer *multipart.Writer) error {
-	return nil
-}
+type testFormProcessor struct{ SavedFileTracker }
 
 func (a *testFormProcessor) ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error {
 	if formName != "token" && !strings.HasPrefix(formName, "file.") && !strings.HasPrefix(formName, "other.") {
@@ -45,10 +40,6 @@ func (a *testFormProcessor) Finalize(ctx context.Context) error {
 	return nil
 }
 
-func (a *testFormProcessor) Name() string {
-	return ""
-}
-
 func TestUploadTempPathRequirement(t *testing.T) {
 	apiResponse := &api.Response{}
 	preparer := &DefaultPreparer{}
@@ -259,6 +250,38 @@ func TestUploadProcessingField(t *testing.T) {
 	require.Equal(t, 500, response.Code)
 }
 
+func TestUploadingMultipleFiles(t *testing.T) {
+	testhelper.ConfigureSecret()
+
+	tempPath, err := ioutil.TempDir("", "uploads")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempPath)
+
+	var buffer bytes.Buffer
+
+	writer := multipart.NewWriter(&buffer)
+	_, err = writer.CreateFormFile("file", "my.file")
+	require.NoError(t, err)
+	_, err = writer.CreateFormFile("file", "my.file")
+	require.NoError(t, err)
+	require.NoError(t, writer.Close())
+
+	httpRequest, err := http.NewRequest("PUT", "/url/path", &buffer)
+	require.NoError(t, err)
+	httpRequest.Header.Set("Content-Type", writer.FormDataContentType())
+
+	response := httptest.NewRecorder()
+	apiResponse := &api.Response{TempPath: tempPath}
+	preparer := &DefaultPreparer{}
+	opts, _, err := preparer.Prepare(apiResponse)
+	require.NoError(t, err)
+
+	HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
+
+	require.Equal(t, 400, response.Code)
+	require.Equal(t, "Uploading multiple files is not allowed\n", response.Body.String())
+}
+
 func TestUploadProcessingFile(t *testing.T) {
 	tempPath, err := ioutil.TempDir("", "uploads")
 	require.NoError(t, err)