From a8cf496e3141fb5640ba4135e36f679ac1b6420a Mon Sep 17 00:00:00 2001
From: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>
Date: Thu, 24 Oct 2024 19:06:14 +0200
Subject: [PATCH] [MM-61213] Limit file size for slack import file uploads
 (#28904)

---
 .../services/slackimport/slackimport.go       | 18 ++++--
 .../services/slackimport/slackimport_test.go  | 57 +++++++++++++++++++
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/server/platform/services/slackimport/slackimport.go b/server/platform/services/slackimport/slackimport.go
index 36786d25ba011..a91771b3defa7 100644
--- a/server/platform/services/slackimport/slackimport.go
+++ b/server/platform/services/slackimport/slackimport.go
@@ -140,6 +140,8 @@ func (si *SlackImporter) SlackImport(rctx request.CTX, fileData multipart.File,
 			log.WriteString(i18n.T("api.slackimport.slack_import.open.app_error", map[string]any{"Filename": file.Name}))
 			return model.NewAppError("SlackImport", "api.slackimport.slack_import.open.app_error", map[string]any{"Filename": file.Name}, "", http.StatusInternalServerError).Wrap(err), log
 		}
+		defer fileReader.Close()
+
 		reader := utils.NewLimitedReaderWithError(fileReader, slackImportMaxFileSize)
 		if file.Name == "channels.json" {
 			publicChannels, err = slackParseChannels(reader, model.ChannelTypeOpen)
@@ -534,8 +536,10 @@ func (si *SlackImporter) slackUploadFile(rctx request.CTX, slackPostFile *slackF
 	}
 	defer openFile.Close()
 
+	// since this is an attachment, we should treat it as a file and apply according limits
+	reader := utils.NewLimitedReaderWithError(openFile, *si.config.FileSettings.MaxFileSize)
 	timestamp := utils.TimeFromMillis(slackConvertTimeStamp(slackTimestamp))
-	uploadedFile, err := si.oldImportFile(rctx, timestamp, openFile, teamId, channelId, userId, filepath.Base(file.Name))
+	uploadedFile, err := si.oldImportFile(rctx, timestamp, reader, teamId, channelId, userId, filepath.Base(file.Name))
 	if err != nil {
 		rctx.Logger().Warn("Slack Import: An error occurred when uploading file.", mlog.String("file_id", slackPostFile.Id), mlog.Err(err))
 		return nil, false
@@ -784,14 +788,18 @@ func (si *SlackImporter) oldImportChannel(rctx request.CTX, channel *model.Chann
 
 func (si *SlackImporter) oldImportFile(rctx request.CTX, timestamp time.Time, file io.Reader, teamId string, channelId string, userId string, fileName string) (*model.FileInfo, error) {
 	buf := bytes.NewBuffer(nil)
-	io.Copy(buf, file)
-	data := buf.Bytes()
-
-	fileInfo, err := si.actions.DoUploadFile(timestamp, teamId, channelId, userId, fileName, data)
+	_, err := io.Copy(buf, file)
 	if err != nil {
 		return nil, err
 	}
 
+	data := buf.Bytes()
+
+	fileInfo, appErr := si.actions.DoUploadFile(timestamp, teamId, channelId, userId, fileName, data)
+	if appErr != nil {
+		return nil, appErr
+	}
+
 	if fileInfo.IsImage() && !fileInfo.IsSvg() {
 		img, imgType, release, err := si.actions.PrepareImage(data)
 		if err != nil {
diff --git a/server/platform/services/slackimport/slackimport_test.go b/server/platform/services/slackimport/slackimport_test.go
index b998e0a263a89..1de12743ccfc6 100644
--- a/server/platform/services/slackimport/slackimport_test.go
+++ b/server/platform/services/slackimport/slackimport_test.go
@@ -4,10 +4,13 @@
 package slackimport
 
 import (
+	"archive/zip"
+	"bytes"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -402,3 +405,57 @@ func TestOldImportChannel(t *testing.T) {
 		_ = importer.oldImportChannel(rctx, ch, sCh, users)
 	})
 }
+
+func TestSlackUploadFile(t *testing.T) {
+	store := &mocks.Store{}
+	config := &model.Config{}
+	config.SetDefaults()
+	defaultLimit := *config.FileSettings.MaxFileSize
+
+	rctx := request.TestContext(t)
+
+	sf := &slackFile{
+		Id:    "testfile",
+		Title: "test-file",
+	}
+
+	buf := new(bytes.Buffer)
+	zipWriter := zip.NewWriter(buf)
+	writer, err := zipWriter.Create("testfile")
+	require.NoError(t, err)
+
+	_, err = writer.Write([]byte(strings.Repeat("a", 100)))
+	require.NoError(t, err)
+
+	err = zipWriter.Close()
+	require.NoError(t, err)
+
+	zipReader, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
+	require.NoError(t, err)
+
+	uploads := map[string]*zip.File{
+		"testfile": zipReader.File[0],
+	}
+
+	t.Run("Should not fail when file is in limits", func(t *testing.T) {
+		importer := New(store, Actions{
+			DoUploadFile: func(_ time.Time, _, _, _, _ string, _ []byte) (*model.FileInfo, *model.AppError) {
+				return &model.FileInfo{}, nil
+			},
+		}, config)
+		_, ok := importer.slackUploadFile(rctx, sf, uploads, "team-id", "channel-id", "user-id", time.Now().String())
+		require.True(t, ok)
+	})
+
+	t.Run("Should fail when file size exceeded", func(t *testing.T) {
+		defer func() {
+			config.FileSettings.MaxFileSize = model.NewPointer(defaultLimit)
+		}()
+
+		config.FileSettings.MaxFileSize = model.NewPointer(int64(10))
+
+		importer := New(store, Actions{}, config)
+		_, ok := importer.slackUploadFile(rctx, sf, uploads, "team-id", "channel-id", "user-id", time.Now().String())
+		require.False(t, ok)
+	})
+}