From fbb05c584fe43e5a36758955a899541807e1f777 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Tue, 16 Jun 2026 11:52:23 +0200 Subject: [PATCH] [MM-69126] Fix custom emoji upload size and GIF frame limits (#36984) * [MM-69126] Fix custom emoji upload size and GIF frame limits * Assert 413 status and error ID in oversized emoji test * Raise max emoji GIF frames to 70 * Enforce emoji GIF frame limit on the direct-write path --- server/channels/api4/emoji.go | 6 +++++ server/channels/api4/emoji_test.go | 35 +++++++++++++++++++++++++++++- server/channels/app/emoji.go | 23 ++++++++++++++++++-- server/i18n/en.json | 4 ++++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/server/channels/api4/emoji.go b/server/channels/api4/emoji.go index 75bd8f8b3ec..d34fd6161be 100644 --- a/server/channels/api4/emoji.go +++ b/server/channels/api4/emoji.go @@ -48,11 +48,17 @@ func createEmoji(c *Context, w http.ResponseWriter, r *http.Request) { return } + r.Body = http.MaxBytesReader(w, r.Body, app.MaxEmojiFileSize) if err := r.ParseMultipartForm(app.MaxEmojiFileSize); err != nil { c.Err = model.NewAppError("createEmoji", "api.emoji.create.parse.app_error", nil, "", http.StatusBadRequest).Wrap(err) return } + if imageFiles := r.MultipartForm.File["image"]; len(imageFiles) > 0 && imageFiles[0].Size > app.MaxEmojiFileSize { + c.Err = model.NewAppError("createEmoji", "api.emoji.create.too_large.app_error", nil, "", http.StatusRequestEntityTooLarge) + return + } + auditRec := c.MakeAuditRecord(model.AuditEventCreateEmoji, model.AuditStatusFail) defer c.LogAuditRec(auditRec) diff --git a/server/channels/api4/emoji_test.go b/server/channels/api4/emoji_test.go index 8dc5b0dc346..b8e59f3c776 100644 --- a/server/channels/api4/emoji_test.go +++ b/server/channels/api4/emoji_test.go @@ -163,8 +163,41 @@ func TestCreateEmoji(t *testing.T) { Name: model.NewId(), } - _, _, err = client.CreateEmoji(context.Background(), emoji, utils.CreateTestAnimatedGif(t, 100, 100, 10000), "image.gif") + _, resp, err = client.CreateEmoji(context.Background(), emoji, utils.CreateTestAnimatedGif(t, 100, 100, 10000), "image.gif") require.Error(t, err, "should fail - emoji is too big") + CheckRequestEntityTooLargeStatus(t, resp) + CheckErrorID(t, err, "api.emoji.create.too_large.app_error") + + // try to create an animated gif with too many frames + emoji = &model.Emoji{ + CreatorId: th.BasicUser.Id, + Name: model.NewId(), + } + + _, resp, err = client.CreateEmoji(context.Background(), emoji, utils.CreateTestAnimatedGif(t, 200, 200, app.MaxEmojiGIFFrames+1), "image.gif") + require.Error(t, err, "should fail - gif has too many frames") + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "api.emoji.upload.too_many_frames.app_error") + + // try to create an animated gif with too many frames that does not need resizing + emoji = &model.Emoji{ + CreatorId: th.BasicUser.Id, + Name: model.NewId(), + } + + _, resp, err = client.CreateEmoji(context.Background(), emoji, utils.CreateTestAnimatedGif(t, app.MaxEmojiWidth, app.MaxEmojiHeight, app.MaxEmojiGIFFrames+1), "image.gif") + require.Error(t, err, "should fail - gif has too many frames") + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "api.emoji.upload.too_many_frames.app_error") + + // try to create an animated gif with exactly the maximum allowed frames + emoji = &model.Emoji{ + CreatorId: th.BasicUser.Id, + Name: model.NewId(), + } + + _, _, err = client.CreateEmoji(context.Background(), emoji, utils.CreateTestAnimatedGif(t, 200, 200, app.MaxEmojiGIFFrames), "image.gif") + require.NoError(t, err, "should succeed - gif has exactly the maximum allowed frames") // try to create an emoji with data that isn't an image emoji = &model.Emoji{ diff --git a/server/channels/app/emoji.go b/server/channels/app/emoji.go index e4b68274f1b..327aa3f05e6 100644 --- a/server/channels/app/emoji.go +++ b/server/channels/app/emoji.go @@ -26,6 +26,7 @@ import ( "github.com/mattermost/mattermost/server/public/shared/request" "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/channels/utils" + "github.com/mattermost/mattermost/server/v8/channels/utils/imgutils" ) const ( @@ -34,6 +35,7 @@ const ( MaxEmojiHeight = 128 MaxEmojiOriginalWidth = 1028 MaxEmojiOriginalHeight = 1028 + MaxEmojiGIFFrames = 70 ) func (a *App) CreateEmoji(rctx request.CTX, sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) { @@ -122,6 +124,24 @@ func (a *App) uploadEmojiImage(rctx request.CTX, id string, filename string, fil return model.NewAppError("uploadEmojiImage", "api.emoji.upload.seek.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + // Enforce the frame limit on every animated GIF, regardless of whether it + // needs resizing, so the cap applies to the direct-write path too. + isGIF := model.NewInfo(filename).MimeType == "image/gif" + if isGIF { + frameCount, err := imgutils.CountGIFFrames(file) + if err != nil { + return model.NewAppError("uploadEmojiImage", "api.emoji.upload.image.app_error", nil, "", http.StatusBadRequest).Wrap(err) + } + if frameCount > MaxEmojiGIFFrames { + return model.NewAppError("uploadEmojiImage", "api.emoji.upload.too_many_frames.app_error", map[string]any{ + "MaxFrames": MaxEmojiGIFFrames, + }, "", http.StatusBadRequest) + } + if _, err = file.Seek(0, io.SeekStart); err != nil { + return model.NewAppError("uploadEmojiImage", "api.emoji.upload.seek.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + } + if config.Width <= MaxEmojiWidth && config.Height <= MaxEmojiHeight { // No need to resize the image _, appErr := a.WriteFile(file, getEmojiImagePath(id)) @@ -131,8 +151,7 @@ func (a *App) uploadEmojiImage(rctx request.CTX, id string, filename string, fil // Create a buffer for the resized image buf := &bytes.Buffer{} - info := model.NewInfo(filename) - if info.MimeType == "image/gif" { + if isGIF { g, err := gif.DecodeAll(file) if err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_decode_error", nil, "", http.StatusBadRequest).Wrap(err) diff --git a/server/i18n/en.json b/server/i18n/en.json index 529c10bf4bc..89564672019 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -2312,6 +2312,10 @@ "id": "api.emoji.upload.seek.app_error", "translation": "Unable to seek to file start." }, + { + "id": "api.emoji.upload.too_many_frames.app_error", + "translation": "Unable to create emoji. Animated GIF must have at most {{.MaxFrames}} frames." + }, { "id": "api.encoding_error", "translation": "Encountered an error encoding JSON for the API response"