Arbitrary file read in Mattermost Boards via import & export board archive

@@ -155,7 +155,7 @@ PLUGIN_PACKAGES += mattermost-plugin-nps-v1.3.3
 PLUGIN_PACKAGES += mattermost-plugin-servicenow-v2.3.4
 PLUGIN_PACKAGES += mattermost-plugin-zoom-v1.8.0
 PLUGIN_PACKAGES += mattermost-plugin-ai-v1.0.0
-PLUGIN_PACKAGES += mattermost-plugin-boards-v9.0.2
+PLUGIN_PACKAGES += mattermost-plugin-boards-v9.0.5
 PLUGIN_PACKAGES += mattermost-plugin-msteams-v2.1.0
 PLUGIN_PACKAGES += mattermost-plugin-user-survey-v1.1.1
 PLUGIN_PACKAGES += mattermost-plugin-mscalendar-v1.3.4

@@ -250,6 +250,11 @@ func (a *API) handlePostBlocks(w http.ResponseWriter, r *http.Request) {
 			hasContents = true
 		}
 
+		if err = block.IsValid(); err != nil {
+			a.errorResponse(w, r, err)
+			return
+		}
+
 		if block.CreateAt < 1 {
 			message := fmt.Sprintf("invalid createAt for block id %s", block.ID)
 			a.errorResponse(w, r, model.NewErrBadRequest(message))

@@ -62,6 +62,10 @@ func (a *App) PatchBlock(blockID string, blockPatch *model.BlockPatch, modifiedB
 }
 
 func (a *App) PatchBlockAndNotify(blockID string, blockPatch *model.BlockPatch, modifiedByID string, disableNotify bool) (*model.Block, error) {
+	if err := model.ValidateBlockPatch(blockPatch); err != nil {
+		return nil, err
+	}
+
 	oldBlock, err := a.store.GetBlock(blockID)
 	if err != nil {
 		return nil, err
@@ -99,6 +103,12 @@ func (a *App) PatchBlockAndNotify(blockID string, blockPatch *model.BlockPatch,
 }
 
 func (a *App) PatchBlocks(teamID string, blockPatches *model.BlockPatchBatch, modifiedByID string) error {
+	for _, patch := range blockPatches.BlockPatches {
+		err := model.ValidateBlockPatch(&patch)
+		if err != nil {
+			return err
+		}
+	}
 	return a.PatchBlocksAndNotify(teamID, blockPatches, modifiedByID, false)
 }
 

@@ -202,14 +202,34 @@ func (a *App) CopyAndUpdateCardFiles(boardID, userID string, blocks []*model.Blo
 	blockPatches := make([]model.BlockPatch, 0)
 	for _, block := range blocks {
 		if block.Type == model.TypeImage || block.Type == model.TypeAttachment {
-			if fileID, ok := block.Fields["fileId"].(string); ok {
-				blockIDs = append(blockIDs, block.ID)
-				blockPatches = append(blockPatches, model.BlockPatch{
-					UpdatedFields: map[string]interface{}{
-						"fileId": newFileNames[fileID],
-					},
-					DeletedFields: []string{"attachmentId"},
-				})
+			if fileID, ok := block.Fields[model.BlockFieldFileId].(string); ok {
+				if err = model.ValidateFileId(fileID); err == nil {
+					blockIDs = append(blockIDs, block.ID)
+					blockPatches = append(blockPatches, model.BlockPatch{
+						UpdatedFields: map[string]interface{}{
+							model.BlockFieldFileId: newFileNames[fileID],
+						},
+						DeletedFields: []string{model.BlockFieldAttachmentId},
+					})
+				} else {
+					errMessage := fmt.Sprintf("invalid characters in block with key: %s, %s", block.Fields[model.BlockFieldFileId], err)
+					return model.NewErrBadRequest(errMessage)
+				}
+			}
+
+			if attachmentID, ok := block.Fields[model.BlockFieldAttachmentId].(string); ok {
+				if err = model.ValidateFileId(attachmentID); err == nil {
+					blockIDs = append(blockIDs, block.ID)
+					blockPatches = append(blockPatches, model.BlockPatch{
+						UpdatedFields: map[string]interface{}{
+							model.BlockFieldAttachmentId: newFileNames[attachmentID],
+						},
+						DeletedFields: []string{model.BlockFieldFileId},
+					})
+				} else {
+					errMessage := fmt.Sprintf("invalid characters in block with key: %s, %s", block.Fields[model.BlockFieldAttachmentId], err)
+					return model.NewErrBadRequest(errMessage)
+				}
 			}
 		}
 	}
@@ -256,6 +276,11 @@ func (a *App) CopyCardFiles(sourceBoardID string, copiedBlocks []*model.Block, a
 			}
 		}
 
+		if err = model.ValidateFileId(fileID); err != nil {
+			errMessage := fmt.Sprintf("Could not validate file ID while duplicating board with fileId: %s", fileID)
+			return nil, model.NewErrBadRequest(errMessage)
+		}
+
 		// create unique filename
 		ext := filepath.Ext(fileID)
 		fileInfoID := utils.NewID(utils.IDTypeNone)

@@ -524,7 +524,52 @@ func TestCopyAndUpdateCardFiles(t *testing.T) {
 		Schema:     1,
 		Type:       "image",
 		Title:      "",
-		Fields:     map[string]interface{}{"fileId": "7fileName.jpg"},
+		Fields:     map[string]interface{}{"fileId": "7fileName1234567890987654321.jpg"},
+		CreateAt:   1680725585250,
+		UpdateAt:   1680725585250,
+		DeleteAt:   0,
+		BoardID:    "boardID",
+	}
+
+	validImageBlock := &model.Block{
+		ID:         "validImageBlock",
+		ParentID:   "c3zqnh6fsu3f4mr6hzq9hizwske",
+		CreatedBy:  "6k6ynxdp47dujjhhojw9nqhmyh",
+		ModifiedBy: "6k6ynxdp47dujjhhojw9nqhmyh",
+		Schema:     1,
+		Type:       "image",
+		Title:      "",
+		Fields:     map[string]interface{}{"fileId": "7xhwgf5r15fr3dryfozf1dmy41r9.png"},
+		CreateAt:   1680725585250,
+		UpdateAt:   1680725585250,
+		DeleteAt:   0,
+		BoardID:    "boardID",
+	}
+
+	invalidShortFileIDBlock := &model.Block{
+		ID:         "invalidShortFileIDBlock",
+		ParentID:   "c3zqnh6fsu3f4mr6hzq9hizwske",
+		CreatedBy:  "6k6ynxdp47dujjhhojw9nqhmyh",
+		ModifiedBy: "6k6ynxdp47dujjhhojw9nqhmyh",
+		Schema:     1,
+		Type:       "image",
+		Title:      "",
+		Fields:     map[string]interface{}{"fileId": "7short.png"},
+		CreateAt:   1680725585250,
+		UpdateAt:   1680725585250,
+		DeleteAt:   0,
+		BoardID:    "boardID",
+	}
+
+	emptyFileBlock := &model.Block{
+		ID:         "emptyFileBlock",
+		ParentID:   "c3zqnh6fsu3f4mr6hzq9hizwske",
+		CreatedBy:  "6k6ynxdp47dujjhhojw9nqhmyh",
+		ModifiedBy: "6k6ynxdp47dujjhhojw9nqhmyh",
+		Schema:     1,
+		Type:       "image",
+		Title:      "",
+		Fields:     map[string]interface{}{"fileId": ""},
 		CreateAt:   1680725585250,
 		UpdateAt:   1680725585250,
 		DeleteAt:   0,
@@ -553,4 +598,70 @@ func TestCopyAndUpdateCardFiles(t *testing.T) {
 
 		assert.NotEqual(t, testPath, imageBlock.Fields["fileId"])
 	})
+
+	t.Run("Valid file ID", func(t *testing.T) {
+		fileInfo := &mm_model.FileInfo{
+			Id:   "validImageBlock",
+			Path: testPath,
+		}
+		th.Store.EXPECT().GetBoard("boardID").Return(&model.Board{ID: "boardID", IsTemplate: false}, nil)
+		th.Store.EXPECT().GetFileInfo("xhwgf5r15fr3dryfozf1dmy41r9").Return(fileInfo, nil)
+		th.Store.EXPECT().SaveFileInfo(fileInfo).Return(nil)
+		th.Store.EXPECT().PatchBlocks(gomock.Any(), "userID").Return(nil)
+
+		mockedFileBackend := &mocks.FileBackend{}
+		th.App.filesBackend = mockedFileBackend
+		mockedFileBackend.On("CopyFile", mock.Anything, mock.Anything).Return(nil)
+
+		err := th.App.CopyAndUpdateCardFiles("boardID", "userID", []*model.Block{validImageBlock}, false)
+		assert.NoError(t, err)
+	})
+
+	t.Run("Invalid file ID length", func(t *testing.T) {
+		th.Store.EXPECT().GetBoard("boardID").Return(&model.Board{ID: "boardID", IsTemplate: false}, nil)
+		err := th.App.CopyAndUpdateCardFiles("boardID", "userID", []*model.Block{invalidShortFileIDBlock}, false)
+		assert.ErrorIs(t, err, model.NewErrBadRequest("Invalid Block ID"))
+	})
+
+	t.Run("Empty file ID", func(t *testing.T) {
+		th.Store.EXPECT().GetBoard("boardID").Return(&model.Board{ID: "boardID", IsTemplate: false}, nil)
+		err := th.App.CopyAndUpdateCardFiles("boardID", "userID", []*model.Block{emptyFileBlock}, false)
+		assert.ErrorIs(t, err, model.NewErrBadRequest("Block ID cannot be empty"))
+	})
+}
+
+func TestCopyCardFiles(t *testing.T) {
+	app := &App{}
+
+	t.Run("ValidFileID", func(t *testing.T) {
+		sourceBoardID := "sourceBoardID"
+		copiedBlocks := []*model.Block{
+			{
+				Type:    model.TypeImage,
+				Fields:  map[string]interface{}{"fileId": "validFileID"},
+				BoardID: "destinationBoardID",
+			},
+		}
+
+		newFileNames, err := app.CopyCardFiles(sourceBoardID, copiedBlocks, false)
+
+		assert.NoError(t, err)
+		assert.NotNil(t, newFileNames)
+	})
+
+	t.Run("InvalidFileID", func(t *testing.T) {
+		sourceBoardID := "sourceBoardID"
+		copiedBlocks := []*model.Block{
+			{
+				Type:    model.TypeImage,
+				Fields:  map[string]interface{}{"fileId": "../../../../../filePath"},
+				BoardID: "destinationBoardID",
+			},
+		}
+
+		newFileNames, err := app.CopyCardFiles(sourceBoardID, copiedBlocks, false)
+
+		assert.Error(t, err)
+		assert.Nil(t, newFileNames)
+	})
 }

@@ -82,6 +82,7 @@ func (a *App) ImportArchive(r io.Reader, opt model.ImportArchiveOptions) error {
 			boardMap[dir] = board
 		default:
 			// import file/image;  dir is the old board id
+
 			board, ok := boardMap[dir]
 			if !ok {
 				a.logger.Warn("skipping orphan image in archive",
@@ -208,6 +209,9 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo
 					if err2 := json.Unmarshal(archiveLine.Data, &block); err2 != nil {
 						return nil, fmt.Errorf("invalid board block in archive line %d: %w", lineNum, err2)
 					}
+					if err := block.IsValid(); err != nil {
+						return nil, err
+					}
 					block.ModifiedBy = userID
 					block.UpdateAt = now
 					board, err := a.blockToBoard(block, opt)
@@ -221,6 +225,9 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo
 					if err2 := json.Unmarshal(archiveLine.Data, &block); err2 != nil {
 						return nil, fmt.Errorf("invalid block in archive line %d: %w", lineNum, err2)
 					}
+					if err := block.IsValid(); err != nil {
+						return nil, err
+					}
 					block.ModifiedBy = userID
 					block.UpdateAt = now
 					block.BoardID = boardID
@@ -263,6 +270,18 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo
 		return nil, fmt.Errorf("error inserting archive blocks: %w", err)
 	}
 
+	if err := a.addUserToNewBoard(boardsAndBlocks, opt, boardMembers); err != nil {
+		return nil, err
+	}
+
+	// find new board id
+	for _, board := range boardsAndBlocks.Boards {
+		return board, nil
+	}
+	return nil, fmt.Errorf("missing board in archive: %w", model.ErrInvalidBoardBlock)
+}
+
+func (a *App) addUserToNewBoard(boardsAndBlocks *model.BoardsAndBlocks, opt model.ImportArchiveOptions, boardMembers []*model.BoardMember) error {
 	// add users to all the new boards (if not the fake system user).
 	for _, board := range boardsAndBlocks.Boards {
 		// make sure an admin user gets added
@@ -272,7 +291,7 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo
 			SchemeAdmin: true,
 		}
 		if _, err2 := a.AddMemberToBoard(adminMember); err2 != nil {
-			return nil, fmt.Errorf("cannot add adminMember to board: %w", err2)
+			return fmt.Errorf("cannot add adminMember to board: %w", err2)
 		}
 		for _, boardMember := range boardMembers {
 			bm := &model.BoardMember{
@@ -287,16 +306,11 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo
 				Synthetic:       boardMember.Synthetic,
 			}
 			if _, err2 := a.AddMemberToBoard(bm); err2 != nil {
-				return nil, fmt.Errorf("cannot add member to board: %w", err2)
+				return fmt.Errorf("cannot add member to board: %w", err2)
 			}
 		}
 	}
-
-	// find new board id
-	for _, board := range boardsAndBlocks.Boards {
-		return board, nil
-	}
-	return nil, fmt.Errorf("missing board in archive: %w", model.ErrInvalidBoardBlock)
+	return nil
 }
 
 // fixBoardsandBlocks allows the caller of `ImportArchive` to modify or filters boards and blocks being

@@ -1,8 +1,6 @@
 package model
 
 import (
-	"errors"
-
 	mmModel "github.com/mattermost/mattermost/server/public/model"
 )
 
@@ -11,8 +9,8 @@ const (
 )
 
 var (
-	errEmptyId   = errors.New("ID cannot be empty")
-	errInvalidId = errors.New("invalid ID")
+	errEmptyId   = NewErrBadRequest("Block ID cannot be empty")
+	errInvalidId = NewErrBadRequest("Invalid Block ID")
 )
 
 func IsValidId(id string) error {

@@ -3,17 +3,23 @@ package model
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
+	"regexp"
 	"strconv"
 	"unicode/utf8"
 
 	"github.com/mattermost/mattermost-plugin-boards/server/services/audit"
+	mmModel "github.com/mattermost/mattermost/server/public/model"
 )
 
 const (
-	BlockTitleMaxBytes  = 65535                  // Maximum size of a TEXT column in MySQL
-	BlockTitleMaxRunes  = BlockTitleMaxBytes / 4 // Assume a worst-case representation
-	BlockFieldsMaxRunes = 800000
+	MinIdLength            = 27
+	BlockTitleMaxBytes     = 65535                  // Maximum size of a TEXT column in MySQL
+	BlockTitleMaxRunes     = BlockTitleMaxBytes / 4 // Assume a worst-case representation
+	BlockFieldsMaxRunes    = 800000
+	BlockFieldFileId       = "fileId"
+	BlockFieldAttachmentId = "attachmentId"
 )
 
 var (
@@ -158,6 +164,80 @@ func (b *Block) IsValid() error {
 		return ErrBlockFieldsSizeLimitExceeded
 	}
 
+	if fileID, ok := b.Fields[BlockFieldFileId].(string); ok {
+		if err = ValidateFileId(fileID); err != nil {
+			return err
+		}
+	}
+
+	if attachmentId, ok := b.Fields[BlockFieldAttachmentId].(string); ok {
+		if err = ValidateFileId(attachmentId); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+var safeInputPattern = regexp.MustCompile(`^[a-zA-Z0-9 _-]+$`)
+
+func ValidateFileId(id string) error {
+	if id == "" {
+		return errEmptyId
+	}
+
+	if len(id) < MinIdLength {
+		return errInvalidId
+	}
+
+	if !mmModel.IsValidId(id[1:28]) {
+		return errInvalidId
+	}
+
+	return nil
+}
+
+func ValidateBlockPatch(patch *BlockPatch) error {
+	// Validate UpdatedFields map
+	if patch.UpdatedFields != nil {
+		if err := validateUpdatedFields(patch.UpdatedFields); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// validateUpdatedFields recursively checks keys and values for unsafe content.
+func validateUpdatedFields(fields map[string]interface{}) error {
+	for key, value := range fields {
+		if !safeInputPattern.MatchString(key) {
+			message := fmt.Sprintf("invalid characters in block with key: %s", key)
+			return NewErrBadRequest(message)
+		}
+
+		if key == BlockFieldFileId {
+			if strVal, ok := value.(string); ok {
+				if err := ValidateFileId(strVal); err != nil {
+					return err
+				}
+			}
+		}
+
+		if key == BlockFieldAttachmentId {
+			if strVal, ok := value.(string); ok {
+				if err := ValidateFileId(strVal); err != nil {
+					return err
+				}
+			}
+		}
+
+		if nestedMap, ok := value.(map[string]interface{}); ok {
+			if err := validateUpdatedFields(nestedMap); err != nil {
+				return err
+			}
+		}
+	}
 	return nil
 }
 

@@ -307,3 +307,124 @@ func TestStampModificationMetadata(t *testing.T) {
 		assert.NotEmpty(t, blocks[0].UpdateAt)
 	})
 }
+func TestValidateBlockPatch(t *testing.T) {
+	t.Run("Should return nil for block patch with valid updated fields", func(t *testing.T) {
+		patch := &BlockPatch{
+			ParentID: nil,
+			Schema:   nil,
+			Type:     nil,
+			Title:    nil,
+			UpdatedFields: map[string]interface{}{
+				"field1": "value1",
+				"field2": 123,
+			},
+			DeletedFields: nil,
+		}
+
+		err := ValidateBlockPatch(patch)
+
+		require.NoError(t, err)
+	})
+
+	t.Run("Should return nil for block patch with valid file ID", func(t *testing.T) {
+		patch := &BlockPatch{
+			ParentID: nil,
+			Schema:   nil,
+			Type:     nil,
+			Title:    nil,
+			UpdatedFields: map[string]interface{}{
+				"fileId": "xhwgf5r15fr3dryfozf1dmy41r9.jpg",
+			},
+			DeletedFields: nil,
+		}
+
+		err := ValidateBlockPatch(patch)
+
+		require.NoError(t, err)
+	})
+
+	t.Run("Should return error for block patch with invalid file ID", func(t *testing.T) {
+		patch := &BlockPatch{
+			ParentID: nil,
+			Schema:   nil,
+			Type:     nil,
+			Title:    nil,
+			UpdatedFields: map[string]interface{}{
+				"fileId": "../../../.../../././././././././filePath",
+			},
+			DeletedFields: nil,
+		}
+
+		err := ValidateBlockPatch(patch)
+
+		require.Error(t, err)
+		require.EqualError(t, err, "Invalid Block ID")
+	})
+
+	t.Run("Should return erro for blok patch with invalid attachment ID", func(t *testing.T) {
+		patch := &BlockPatch{
+			ParentID: nil,
+			Schema:   nil,
+			Type:     nil,
+			Title:    nil,
+			UpdatedFields: map[string]interface{}{
+				"attchmentId": "../../../.../../././././././././filePath",
+			},
+			DeletedFields: nil,
+		}
+
+		err := ValidateBlockPatch(patch)
+
+		require.Error(t, err)
+		require.EqualError(t, err, "Invalid Block ID")
+	})
+
+	t.Run("Should return error for block patch with nested UpdatedFields", func(t *testing.T) {
+		patch := &BlockPatch{
+			ParentID: nil,
+			Schema:   nil,
+			Type:     nil,
+			Title:    nil,
+			UpdatedFields: map[string]interface{}{
+				"0": "value1",
+				"1": map[string]interface{}{
+					"fileId": "../../../.../../././././././././filePath",
+				},
+			},
+			DeletedFields: nil,
+		}
+
+		err := ValidateBlockPatch(patch)
+
+		require.Error(t, err)
+		require.EqualError(t, err, "Invalid Block ID")
+	})
+}
+func TestValidateFileId(t *testing.T) {
+	t.Run("Should return nil for valid file ID", func(t *testing.T) {
+		fileID := "xhwgf5r15fr3dryfozf1dmy41r9.jpg"
+		err := ValidateFileId(fileID)
+		require.NoError(t, err)
+	})
+
+	t.Run("Should return error for empty file ID", func(t *testing.T) {
+		fileID := ""
+		err := ValidateFileId(fileID)
+		require.Error(t, err)
+		require.EqualError(t, err, "Block ID cannot be empty")
+	})
+
+	t.Run("Should return error for invalid file ID length", func(t *testing.T) {
+		fileID := "xhwgf5r15fr3dryfozf1dmy41r9"
+		err := ValidateFileId(fileID)
+		require.Error(t, err)
+		require.EqualError(t, err, "Invalid Block ID")
+	})
+
+	t.Run("Should return error for file ID with invalid characters", func(t *testing.T) {
+		fileID := "../../../.../../././././././././filePath"
+		err := ValidateFileId(fileID)
+		require.Error(t, err)
+		require.EqualError(t, err, "Invalid Block ID")
+	})
+}