From eb8c99fe9cd4417e20b4227ecf4eb18b4cdfc88c Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Fri, 13 Feb 2026 14:09:32 +0200 Subject: [PATCH] Add fileSize limit to extractors (#35200) (#35280) Automatic Merge --- server/channels/app/file.go | 1 + .../platform/services/docextractor/archive.go | 19 +++-- .../platform/services/docextractor/combine.go | 4 +- .../services/docextractor/docextractor.go | 3 +- .../docextractor/docextractor_test.go | 73 ++++++++++++++++++- .../services/docextractor/documents.go | 2 +- .../services/docextractor/interface.go | 2 +- .../services/docextractor/mmpreview.go | 4 +- server/platform/services/docextractor/pdf.go | 2 +- .../services/docextractor/pdf_test.go | 6 +- .../platform/services/docextractor/plain.go | 2 +- .../services/docextractor/plain_test.go | 10 +-- 12 files changed, 104 insertions(+), 24 deletions(-) diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 075dd543fc..254b4e7d04 100644 --- a/server/channels/app/file.go +++ b/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") diff --git a/server/platform/services/docextractor/archive.go b/server/platform/services/docextractor/archive.go index 72ba17f03d..1fd89f4472 100644 --- a/server/platform/services/docextractor/archive.go +++ b/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 + " ") } diff --git a/server/platform/services/docextractor/combine.go b/server/platform/services/docextractor/combine.go index 474b53faee..277fe5772f 100644 --- a/server/platform/services/docextractor/combine.go +++ b/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 diff --git a/server/platform/services/docextractor/docextractor.go b/server/platform/services/docextractor/docextractor.go index 557d03303e..a4de967eb3 100644 --- a/server/platform/services/docextractor/docextractor.go +++ b/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 } diff --git a/server/platform/services/docextractor/docextractor_test.go b/server/platform/services/docextractor/docextractor_test.go index 3ba93876fa..fbbe27868d 100644 --- a/server/platform/services/docextractor/docextractor_test.go +++ b/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) + } + }) + } +} diff --git a/server/platform/services/docextractor/documents.go b/server/platform/services/docextractor/documents.go index 1e06fa6163..a0c11d9606 100644 --- a/server/platform/services/docextractor/documents.go +++ b/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 = "" diff --git a/server/platform/services/docextractor/interface.go b/server/platform/services/docextractor/interface.go index 582fb0b502..9bf5503917 100644 --- a/server/platform/services/docextractor/interface.go +++ b/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 } diff --git a/server/platform/services/docextractor/mmpreview.go b/server/platform/services/docextractor/mmpreview.go index 51a261e233..400cc495ff 100644 --- a/server/platform/services/docextractor/mmpreview.go +++ b/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) { diff --git a/server/platform/services/docextractor/pdf.go b/server/platform/services/docextractor/pdf.go index 67ba8ad274..2c245c3d66 100644 --- a/server/platform/services/docextractor/pdf.go +++ b/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 = "" diff --git a/server/platform/services/docextractor/pdf_test.go b/server/platform/services/docextractor/pdf_test.go index e191ed5292..c3a993d4c5 100644 --- a/server/platform/services/docextractor/pdf_test.go +++ b/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) } diff --git a/server/platform/services/docextractor/plain.go b/server/platform/services/docextractor/plain.go index 8877c20b34..b6fff6bd69 100644 --- a/server/platform/services/docextractor/plain.go +++ b/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) diff --git a/server/platform/services/docextractor/plain_test.go b/server/platform/services/docextractor/plain_test.go index 4108f22050..8e1865e525 100644 --- a/server/platform/services/docextractor/plain_test.go +++ b/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) }