Automated cherry pick of #220

server/api/archive.go

@@ -4,6 +4,7 @@
 package api
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -156,9 +157,18 @@ func (a *API) handleArchiveImport(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if a.app.GetConfig().MaxFileSize > 0 {
+		r.Body = http.MaxBytesReader(w, r.Body, a.app.GetConfig().MaxFileSize)
+	}
+
 	file, handle, err := r.FormFile(UploadFormFileKey)
 	if err != nil {
-		fmt.Fprintf(w, "%v", err)
+		var maxBytesErr *http.MaxBytesError
+		if errors.As(err, &maxBytesErr) {
+			a.errorResponse(w, r, model.ErrRequestEntityTooLarge)
+			return
+		}
+		a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
 		return
 	}
 	defer file.Close()

server/api/archive_test.go

@@ -0,0 +1,133 @@
+// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package api
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/gorilla/mux"
+	"github.com/mattermost/mattermost-plugin-boards/server/app"
+	"github.com/mattermost/mattermost-plugin-boards/server/auth"
+	"github.com/mattermost/mattermost-plugin-boards/server/model"
+	"github.com/mattermost/mattermost-plugin-boards/server/services/config"
+	"github.com/mattermost/mattermost-plugin-boards/server/services/metrics"
+	"github.com/mattermost/mattermost-plugin-boards/server/services/permissions/mmpermissions"
+	mmpermissionsMocks "github.com/mattermost/mattermost-plugin-boards/server/services/permissions/mmpermissions/mocks"
+	permissionsMocks "github.com/mattermost/mattermost-plugin-boards/server/services/permissions/mocks"
+	"github.com/mattermost/mattermost-plugin-boards/server/services/store/mockstore"
+	"github.com/mattermost/mattermost-plugin-boards/server/services/webhook"
+	"github.com/mattermost/mattermost-plugin-boards/server/ws"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/shared/mlog"
+	"github.com/mattermost/mattermost/server/v8/platform/shared/filestore/mocks"
+)
+
+func setupArchiveImportAPI(t *testing.T, maxFileSize int64) (*API, func()) {
+	t.Helper()
+
+	ctrl := gomock.NewController(t)
+	cfg := config.Configuration{MaxFileSize: maxFileSize}
+	store := mockstore.NewMockStore(ctrl)
+	filesBackend := &mocks.FileBackend{}
+	authService := auth.New(&cfg, store, nil)
+	logger, _ := mlog.NewLogger()
+	wsserver := ws.NewServer(authService, logger, store)
+	webhookClient := webhook.NewClient(&cfg, logger)
+	metricsService := metrics.NewMetrics(metrics.InstanceInfo{})
+
+	permStore := permissionsMocks.NewMockStore(ctrl)
+	pluginAPI := mmpermissionsMocks.NewMockAPI(ctrl)
+	pluginAPI.EXPECT().HasPermissionToTeam(gomock.Any(), gomock.Any(), model.PermissionViewTeam).Return(true)
+	store.EXPECT().GetUserByID(gomock.Any()).Return(&model.User{ID: "user", IsGuest: false}, nil)
+	permissions := mmpermissions.New(permStore, pluginAPI, mlog.CreateConsoleTestLogger(t))
+
+	testApp := app.New(&cfg, wsserver, app.Services{
+		Auth:             authService,
+		Store:            store,
+		FilesBackend:     filesBackend,
+		Webhook:          webhookClient,
+		Metrics:          metricsService,
+		Logger:           logger,
+		SkipTemplateInit: true,
+		Permissions:      permissions,
+	})
+
+	api := NewAPI(testApp, "", "", permissions, mlog.CreateConsoleTestLogger(t), nil)
+
+	tearDown := func() {
+		testApp.Shutdown()
+		if logger != nil {
+			_ = logger.Shutdown()
+		}
+	}
+
+	return api, tearDown
+}
+
+func archiveImportRequest(t *testing.T, teamID string, body io.Reader, contentType string) *http.Request {
+	t.Helper()
+
+	req := httptest.NewRequest(http.MethodPost, "/teams/"+teamID+"/archive/import", body)
+	req = mux.SetURLVars(req, map[string]string{"teamID": teamID})
+	if contentType != "" {
+		req.Header.Set("Content-Type", contentType)
+	}
+
+	session := &model.Session{UserID: "user"}
+	ctx := context.WithValue(req.Context(), sessionContextKey, session)
+	return req.WithContext(ctx)
+}
+
+func TestHandleArchiveImportFormErrors(t *testing.T) {
+	const teamID = "abcdefghijklmnopqrstuvwxyz"
+
+	t.Run("returns 413 for oversized multipart body", func(t *testing.T) {
+		api, tearDown := setupArchiveImportAPI(t, 1)
+		defer tearDown()
+
+		var body bytes.Buffer
+		writer := multipart.NewWriter(&body)
+		part, err := writer.CreateFormFile(UploadFormFileKey, "archive.boardarchive")
+		require.NoError(t, err)
+		_, err = part.Write([]byte("too-large"))
+		require.NoError(t, err)
+		require.NoError(t, writer.Close())
+
+		req := archiveImportRequest(t, teamID, &body, writer.FormDataContentType())
+		w := httptest.NewRecorder()
+
+		api.handleArchiveImport(w, req)
+
+		res := w.Result()
+		defer res.Body.Close()
+		require.Equal(t, http.StatusRequestEntityTooLarge, res.StatusCode)
+		require.Equal(t, "application/json", res.Header.Get("Content-Type"))
+		b, readErr := io.ReadAll(res.Body)
+		require.NoError(t, readErr)
+		require.Contains(t, string(b), "entity too large")
+	})
+
+	t.Run("returns 400 for malformed upload", func(t *testing.T) {
+		api, tearDown := setupArchiveImportAPI(t, 0)
+		defer tearDown()
+
+		req := archiveImportRequest(t, teamID, bytes.NewReader([]byte("not-a-multipart-body")), "text/plain")
+		w := httptest.NewRecorder()
+
+		api.handleArchiveImport(w, req)
+
+		res := w.Result()
+		defer res.Body.Close()
+		require.Equal(t, http.StatusBadRequest, res.StatusCode)
+		require.Equal(t, "application/json", res.Header.Get("Content-Type"))
+	})
+}

server/app/import.go

@@ -68,9 +68,11 @@ func (a *App) ImportArchive(r io.Reader, opt model.ImportArchiveOptions) error {
 		dir, filename := filepath.Split(hdr.Name)
 		dir = path.Clean(dir)
 
+		maxEntry := a.effectiveArchiveEntryMaxSize()
+
 		switch filename {
 		case "version.json":
-			ver, errVer := parseVersionFile(zr)
+			ver, errVer := parseVersionFile(zr, maxEntry)
 			if errVer != nil {
 				return errVer
 			}
@@ -92,12 +94,28 @@ func (a *App) ImportArchive(r io.Reader, opt model.ImportArchiveOptions) error {
 					mlog.String("dir", dir),
 					mlog.String("filename", filename),
 				)
+				if err := discardLimited(zr, maxEntry); err != nil {
+					return fmt.Errorf("cannot skip orphan file %s: %w", filename, err)
+				}
 				continue
 			}
-			newFileName, err := a.SaveFile(zr, opt.TeamID, board.ID, filename, board.IsTemplate)
+			fileReader := newLimitedReader(zr, maxEntry)
+			newFileName, err := a.SaveFile(fileReader, opt.TeamID, board.ID, filename, board.IsTemplate)
 			if err != nil {
 				return fmt.Errorf("cannot import file %s for board %s: %w", filename, dir, err)
 			}
+			if limitedReaderExceeded(fileReader) {
+				filePath, pathErr := getDestinationFilePath(board.IsTemplate, opt.TeamID, board.ID, newFileName)
+				if pathErr == nil {
+					if removeErr := a.filesBackend.RemoveFile(filePath); removeErr != nil {
+						a.logger.Warn("failed to remove oversized import file",
+							mlog.String("path", filePath),
+							mlog.Err(removeErr),
+						)
+					}
+				}
+				return fmt.Errorf("cannot import file %s for board %s: %w", filename, dir, errSizeLimitExceeded)
+			}
 			fileMap[filename] = newFileName
 
 			a.logger.Debug("import archive file",
@@ -472,15 +490,48 @@ func arrayMapsValue(m map[string]interface{}, key string) ([]map[string]interfac
 	return arr, true
 }
 
-func parseVersionFile(r io.Reader) (int, error) {
-	file, err := io.ReadAll(r)
+func newLimitedReader(r io.Reader, limit int64) *io.LimitedReader {
+	return &io.LimitedReader{R: r, N: limit + 1}
+}
+
+func limitedReaderExceeded(lr *io.LimitedReader) bool {
+	return lr.N <= 0
+}
+
+func (a *App) effectiveArchiveEntryMaxSize() int64 {
+	maxEntry := int64(importMaxFileSize)
+	if a.config != nil {
+		if cfgMax := a.config.MaxFileSize; cfgMax > 0 && cfgMax < maxEntry {
+			maxEntry = cfgMax
+		}
+	}
+	return maxEntry
+}
+
+func parseVersionFile(r io.Reader, maxSize int64) (int, error) {
+	lr := newLimitedReader(r, maxSize)
+	file, err := io.ReadAll(lr)
 	if err != nil {
 		return 0, fmt.Errorf("cannot read version.json: %w", err)
 	}
+	if limitedReaderExceeded(lr) {
+		return 0, fmt.Errorf("cannot read version.json: %w", errSizeLimitExceeded)
+	}
 
 	var header model.ArchiveHeader
 	if err := json.Unmarshal(file, &header); err != nil {
 		return 0, fmt.Errorf("cannot parse version.json: %w", err)
 	}
 	return header.Version, nil
 }
+
+func discardLimited(r io.Reader, limit int64) error {
+	lr := newLimitedReader(r, limit)
+	if _, err := io.Copy(io.Discard, lr); err != nil {
+		return err
+	}
+	if limitedReaderExceeded(lr) {
+		return errSizeLimitExceeded
+	}
+	return nil
+}