Add fileSize limit to extractors (#35200) (#35280)

Automatic Merge

server/channels/app/file.go

@@ -1587,6 +1587,7 @@ func (a *App) ExtractContentFromFileInfo(rctx request.CTX, fileInfo *model.FileI
 	defer file.Close()
 	text, err := docextractor.Extract(rctx.Logger(), fileInfo.Name, file, docextractor.ExtractSettings{
 		ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion,
+		MaxFileSize:      *a.Config().FileSettings.MaxFileSize,
 	})
 	if err != nil {
 		return errors.Wrap(err, "failed to extract file content")

server/platform/services/docextractor/archive.go

@@ -14,6 +14,8 @@ import (
 	"strings"
 
 	"github.com/mholt/archives"
+
+	"github.com/mattermost/mattermost/server/v8/channels/utils"
 )
 
 type archiveExtractor struct {
@@ -38,7 +40,7 @@ func getExtAlsoTarGz(name string) string {
 	return filepath.Ext(name)
 }
 
-func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker) (string, error) {
+func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker, maxFileSize int64) (string, error) {
 	ext := getExtAlsoTarGz(name)
 
 	// Create a temporary file, using `*` control the random component while preserving the extension.
@@ -81,12 +83,19 @@ func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker) (string, error
 			}
 			defer file.Close()
 
-			data, err := io.ReadAll(file)
-			if err != nil {
-				return err
+			// Limit the size of decompressed archive entries to prevent
+			// memory exhaustion from zip bombs or other malicious archives.
+			var reader io.Reader = file
+			if maxFileSize > 0 {
+				reader = utils.NewLimitedReaderWithError(file, maxFileSize)
 			}
 
-			subtext, extractErr := ae.SubExtractor.Extract(filename, bytes.NewReader(data))
+			data, err := io.ReadAll(reader)
+			if err != nil {
+				return fmt.Errorf("error reading archive entry %s: %w", path, err)
+			}
+
+			subtext, extractErr := ae.SubExtractor.Extract(filename, bytes.NewReader(data), maxFileSize)
 			if extractErr == nil {
 				text.WriteString(subtext + " ")
 			}

server/platform/services/docextractor/combine.go

@@ -31,11 +31,11 @@ func (ce *combineExtractor) Match(filename string) bool {
 	return false
 }
 
-func (ce *combineExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
+func (ce *combineExtractor) Extract(filename string, r io.ReadSeeker, maxFileSize int64) (string, error) {
 	for _, extractor := range ce.SubExtractors {
 		if extractor.Match(filename) {
 			r.Seek(0, io.SeekStart)
-			text, err := extractor.Extract(filename, r)
+			text, err := extractor.Extract(filename, r, maxFileSize)
 			if err != nil {
 				ce.logger.Warn("Unable to extract file content", mlog.String("file_name", filename), mlog.String("extractor", extractor.Name()), mlog.Err(err))
 				continue

server/platform/services/docextractor/docextractor.go

@@ -12,6 +12,7 @@ import (
 // ExtractSettings defines the features enabled/disable during the document text extraction.
 type ExtractSettings struct {
 	ArchiveRecursion bool
+	MaxFileSize      int64
 	MMPreviewURL     string
 	MMPreviewSecret  string
 }
@@ -44,7 +45,7 @@ func ExtractWithExtraExtractors(logger mlog.LoggerIFace, filename string, r io.R
 	enabledExtractors.Add(&plainExtractor{})
 
 	if enabledExtractors.Match(filename) {
-		return enabledExtractors.Extract(filename, r)
+		return enabledExtractors.Extract(filename, r, settings.MaxFileSize)
 	}
 	return "", nil
 }

server/platform/services/docextractor/docextractor_test.go

@@ -172,7 +172,7 @@ func (te *customTestPdfExtractor) Match(filename string) bool {
 	return strings.HasSuffix(filename, ".pdf")
 }
 
-func (te *customTestPdfExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
+func (te *customTestPdfExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) {
 	return "this is a text generated content", nil
 }
 
@@ -186,7 +186,7 @@ func (te *failingExtractor) Match(filename string) bool {
 	return true
 }
 
-func (te *failingExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
+func (te *failingExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) {
 	return "", errors.New("this always fail")
 }
 
@@ -212,3 +212,72 @@ func TestExtractWithExtraExtractors(t *testing.T) {
 		assert.Contains(t, text, "contains")
 	})
 }
+
+func TestArchiveMaxFileSize(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		file           string
+		recursion      bool
+		limit          int64
+		expectContains []string
+		expectMissing  []string
+	}{
+		{
+			name:           "Zip with recursion and large limit extracts fully",
+			file:           "Fake_Team_Import.zip",
+			recursion:      true,
+			limit:          10 * 1024 * 1024,
+			expectContains: []string{"purpose", "announcements"},
+		},
+		{
+			name:          "Zip with recursion and tiny limit rejects oversized entries",
+			file:          "Fake_Team_Import.zip",
+			recursion:     true,
+			limit:         1,
+			expectMissing: []string{"purpose", "announcements"},
+		},
+		{
+			name:           "Zip with recursion and zero limit means unlimited",
+			file:           "Fake_Team_Import.zip",
+			recursion:      true,
+			limit:          0,
+			expectContains: []string{"purpose", "announcements"},
+		},
+		{
+			name:           "Zip without recursion lists paths regardless of limit",
+			file:           "Fake_Team_Import.zip",
+			recursion:      false,
+			limit:          1,
+			expectContains: []string{"channels"},
+		},
+		{
+			name:          "Tar.gz with recursion and tiny limit rejects oversized entries",
+			file:          "Fake_Team_Import.tar.gz",
+			recursion:     true,
+			limit:         1,
+			expectMissing: []string{"purpose", "announcements"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			data, err := testutils.ReadTestFile(tc.file)
+			require.NoError(t, err)
+
+			settings := ExtractSettings{ArchiveRecursion: tc.recursion, MaxFileSize: tc.limit}
+			text, err := Extract(mlog.CreateConsoleTestLogger(t), tc.file, bytes.NewReader(data), settings)
+			require.NoError(t, err)
+
+			for _, s := range tc.expectContains {
+				assert.Contains(t, text, s)
+			}
+			for _, s := range tc.expectMissing {
+				assert.NotContains(t, text, s)
+			}
+		})
+	}
+}

server/platform/services/docextractor/documents.go

@@ -36,7 +36,7 @@ func (de *documentExtractor) Match(filename string) bool {
 	return ok
 }
 
-func (de *documentExtractor) Extract(filename string, r io.ReadSeeker) (out string, outErr error) {
+func (de *documentExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (out string, outErr error) {
 	defer func() {
 		if r := recover(); r != nil {
 			out = ""

server/platform/services/docextractor/interface.go

@@ -10,6 +10,6 @@ import (
 // Extractors define the interface needed to extract file content
 type Extractor interface {
 	Match(filename string) bool
-	Extract(filename string, file io.ReadSeeker) (string, error)
+	Extract(filename string, file io.ReadSeeker, maxFileSize int64) (string, error)
 	Name() string
 }

server/platform/services/docextractor/mmpreview.go

@@ -45,7 +45,7 @@ func (mpe *mmPreviewExtractor) Match(filename string) bool {
 	return mmpreviewSupportedExtensions[extension]
 }
 
-func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker) (string, error) {
+func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker, maxFileSize int64) (string, error) {
 	b, w, err := createMultipartFormData("file", filename, file)
 	if err != nil {
 		return "", errors.Wrap(err, "Unable to generate file preview using mmpreview.")
@@ -70,7 +70,7 @@ func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker) (str
 	if err != nil {
 		return "", errors.Wrap(err, "unable to read the response from mmpreview")
 	}
-	return mpe.pdfExtractor.Extract(filename, bytes.NewReader(data))
+	return mpe.pdfExtractor.Extract(filename, bytes.NewReader(data), maxFileSize)
 }
 
 func createMultipartFormData(fieldName, fileName string, fileData io.ReadSeeker) (bytes.Buffer, *multipart.Writer, error) {

server/platform/services/docextractor/pdf.go

@@ -29,7 +29,7 @@ func (pe *pdfExtractor) Match(filename string) bool {
 	return supportedExtensions[extension]
 }
 
-func (pe *pdfExtractor) Extract(filename string, r io.ReadSeeker) (out string, outErr error) {
+func (pe *pdfExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (out string, outErr error) {
 	defer func() {
 		if r := recover(); r != nil {
 			out = ""

server/platform/services/docextractor/pdf_test.go

@@ -14,7 +14,7 @@ import (
 
 func TestPdfEmptyFile(t *testing.T) {
 	extractor := pdfExtractor{}
-	_, err := extractor.Extract("test.pdf", bytes.NewReader([]byte{}))
+	_, err := extractor.Extract("test.pdf", bytes.NewReader([]byte{}), 0)
 	require.Error(t, err)
 }
 
@@ -23,7 +23,7 @@ func TestPdfFile(t *testing.T) {
 	contentText := "This is a simple document that contains some text."
 	content, err := testutils.ReadTestFile("sample-doc.pdf")
 	require.NoError(t, err)
-	extractedText, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content))
+	extractedText, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 0)
 	require.NoError(t, err)
 	require.Equal(t, contentText, extractedText)
 }
@@ -32,6 +32,6 @@ func TestWrongPdfFile(t *testing.T) {
 	extractor := pdfExtractor{}
 	content, err := testutils.ReadTestFile("sample-doc.docx")
 	require.NoError(t, err)
-	_, err = extractor.Extract("sample-doc.pdf", bytes.NewReader(content))
+	_, err = extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 0)
 	require.Error(t, err)
 }

server/platform/services/docextractor/plain.go

@@ -19,7 +19,7 @@ func (pe *plainExtractor) Match(filename string) bool {
 	return true
 }
 
-func (pe *plainExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
+func (pe *plainExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) {
 	// This detects any visible character plus any whitespace
 	validRanges := append(unicode.GraphicRanges, unicode.White_Space)
 

server/platform/services/docextractor/plain_test.go

@@ -13,7 +13,7 @@ import (
 
 func TestPlainEmptyFile(t *testing.T) {
 	extractor := plainExtractor{}
-	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte{}))
+	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte{}), 0)
 	require.NoError(t, err)
 	require.Equal(t, "", extractedText)
 }
@@ -21,7 +21,7 @@ func TestPlainEmptyFile(t *testing.T) {
 func TestPlainTextSmallFile(t *testing.T) {
 	extractor := plainExtractor{}
 	content := strings.Repeat("test \n", 5)
-	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)))
+	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)), 0)
 	require.NoError(t, err)
 	require.Equal(t, content, extractedText)
 }
@@ -29,7 +29,7 @@ func TestPlainTextSmallFile(t *testing.T) {
 func TestPlainBigFile(t *testing.T) {
 	extractor := plainExtractor{}
 	content := strings.Repeat("test \n", 1000)
-	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)))
+	extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)), 0)
 	require.NoError(t, err)
 	require.Equal(t, content, extractedText)
 }
@@ -38,7 +38,7 @@ func TestSmallBinaryFile(t *testing.T) {
 	extractor := plainExtractor{}
 	notUTF8Char := byte(0x7)
 	content := bytes.Repeat([]byte{notUTF8Char}, 1000)
-	extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content))
+	extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content), 0)
 	require.NoError(t, err)
 	require.Equal(t, "", extractedText)
 }
@@ -47,7 +47,7 @@ func TestBigBinaryFile(t *testing.T) {
 	extractor := plainExtractor{}
 	notUTF8Char := byte(0x7)
 	content := bytes.Repeat([]byte{notUTF8Char}, 10000)
-	extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content))
+	extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content), 0)
 	require.NoError(t, err)
 	require.Equal(t, "", extractedText)
 }